Changeset 94785 in webkit

Timestamp:

Sep 8, 2011 12:58:32 PM (13 years ago)

Author:

robert@webkit.org

Message:

Elements with position:absolute don't move to correct position after images load
​https://bugs.webkit.org/show_bug.cgi?id=54611

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/block/positioning/absolute-layout-after-image-load.html

In the test the 'label' block is an absolutely positioned child of an inline flow. So during layout,
this RenderBlock::layoutPositionedObjects fails to dirty it for rendering because it requires
the parent to be a BlockFlow. The code to do this was introduced in ​http://trac.webkit.org/changeset/8284.
There doesn't seem to be a good reason for requiring a BlockFlow, so remove the check.

Note: Although the issue is encountered only on first load without a fragment identifier, it
happens reliably when you include the fragment identifier in the url (#Footnote_1). This is so
because scrolling to the fragment always happens before the image has loaded, rendering the page
and clearing the initial dirty bits in the positioned element's renderer. When the image finally
loads in this scenario, the positioned element is otherwise clean and relies on the above code to get
re-rendered.

Note: This was originally landed in r94755 but positioned-float-layout-after-image-load.html exposed

an ASSERT bug, unrelated to this change, and was rolled out. That issue is tracked separately
in bug 67759.

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::layoutPositionedObjects): remove the check for r->parent()->isBlockFlow() when

deciding whether to mark children for layout

LayoutTests:

• fast/block/positioning/absolute-layout-after-image-load-expected.txt: Added.
• fast/block/positioning/absolute-layout-after-image-load.html: Added.
• fast/block/positioning/resources/absolute-layout-after-image-load-2.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt (added)
• LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html (added)
• LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html (added)
• LayoutTests/platform/chromium/test_expectations.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-09-03  Robert Hogan  <robert@webkit.org>
+
+        Elements with position:absolute don't move to correct position after images load
+        https://bugs.webkit.org/show_bug.cgi?id=54611
+
+        Reviewed by Simon Fraser.
+
+        * fast/block/positioning/absolute-layout-after-image-load-expected.txt: Added.
+        * fast/block/positioning/absolute-layout-after-image-load.html: Added.
+        * fast/block/positioning/resources/absolute-layout-after-image-load-2.html: Added.
+
 2011-09-08  Roland Steiner  <rolandsteiner@chromium.org>
 

trunk/LayoutTests/platform/chromium/test_expectations.txt

@@ -3715 +3715 @@
 BUG_REED : fast/canvas/quadraticCurveTo.xml = IMAGE
 BUG_REED : fast/canvas/canvas-lineWidth.html = TEXT
-
-BUGWK67769 DEBUG : fast/block/positioning/positioned-float-layout-after-image-load.html = CRASH

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-09-03  Robert Hogan  <robert@webkit.org>
+
+        Elements with position:absolute don't move to correct position after images load
+        https://bugs.webkit.org/show_bug.cgi?id=54611
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/block/positioning/absolute-layout-after-image-load.html
+
+        In the test the 'label' block is an absolutely positioned child of an inline flow. So during layout, 
+        this RenderBlock::layoutPositionedObjects fails to dirty it for rendering because it requires 
+        the parent to be a BlockFlow. The code to do this was introduced in http://trac.webkit.org/changeset/8284. 
+        There doesn't seem to be a good reason for requiring a BlockFlow, so remove the check. 
+
+        Note: Although the issue is encountered only on first load without a fragment identifier, it 
+        happens reliably when you include the fragment identifier in the url (#Footnote_1). This is so 
+        because scrolling to the fragment always happens before the image has loaded, rendering the page 
+        and clearing the initial dirty bits in the positioned element's renderer. When the image finally 
+        loads in this scenario, the positioned element is otherwise clean and relies on the above code to get 
+        re-rendered.
+
+        Note: This was originally landed in r94755 but positioned-float-layout-after-image-load.html exposed
+              an ASSERT bug, unrelated to this change, and was rolled out. That issue is tracked separately
+              in bug 67759.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::layoutPositionedObjects): remove the check for r->parent()->isBlockFlow() when 
+                                                         deciding whether to mark children for layout
+
 2011-09-08  Roland Steiner  <rolandsteiner@chromium.org>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -2245 +2245 @@
         // objects that are positioned implicitly like this.  Such objects are rare, and so in typical DHTML menu usage (where everything is
         // positioned explicitly) this should not incur a performance penalty.
-        if (relayoutChildren || (r->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && r->parent() != this && r->parent()->isBlockFlow()))
+        if (relayoutChildren || (r->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && r->parent() != this))
             r->setChildNeedsLayout(true, false);