Elements with position:absolute don't move to correct position after images load
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Sep 2011 18:59:55 +0000 (18:59 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Sep 2011 18:59:55 +0000 (18:59 +0000)
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
      fast/block/positioning/positioned-float-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. Do the same
for positioned floats in RenderBlock::positionedFloatsNeedRelayout(), although currently layoutPositionedObjects()
takes care of it this at least ensures no regression in future.

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.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::layoutPositionedObjects): remove the check for r->parent()->isBlockFlow() when
                                                 deciding whether to mark children for layout
(WebCore::RenderBlock::positionedFloatsNeedRelayout): ditto

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.
* fast/block/positioning/positioned-float-layout-after-image-load-expected.txt: Added.
* fast/block/positioning/positioned-float-layout-after-image-load.html: Added.
* fast/block/positioning/resources/positioned-float-layout-after-image-load-2.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@94695 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html [new file with mode: 0644]
LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load.html [new file with mode: 0644]
LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html [new file with mode: 0644]
LayoutTests/fast/block/positioning/resources/positioned-float-layout-after-image-load-2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index 00dbfa9a7cd790c326471b24fcea6a0160f7f7a1..b257ef7d241f3228e9461a58160afd4751318a95 100644 (file)
@@ -1,3 +1,17 @@
+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.
+        * fast/block/positioning/positioned-float-layout-after-image-load-expected.txt: Added.
+        * fast/block/positioning/positioned-float-layout-after-image-load.html: Added.
+        * fast/block/positioning/resources/positioned-float-layout-after-image-load-2.html: Added.
+
 2011-09-07  Pavel Podivilov  <podivilov@chromium.org>
 
         Unreviewed, build fix after r94674.
diff --git a/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt b/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt
new file mode 100644 (file)
index 0000000..64e2ca5
--- /dev/null
@@ -0,0 +1,3 @@
+
+[1]
+SUCCESS
diff --git a/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html b/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html
new file mode 100644 (file)
index 0000000..b6f0b95
--- /dev/null
@@ -0,0 +1,21 @@
+<html>
+<head>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+     }
+
+    //   https://bugs.webkit.org/show_bug.cgi?id=54611
+    //   When navigating to absolute-layout-after-image-load-2.html#anchor1 directly, the anchor should be positioned below 
+    //   the image on first load. The test is sensitive to caching of the image, so you should reload 
+    //   absolute-layout-after-image-load-2.html#anchor1 if testing manually.
+
+    function test(){
+        setTimeout(location.assign("resources/absolute-layout-after-image-load-2.html#anchor1"),0);
+    }
+</script>
+</head>
+<body onload="test();">
+</body></html>
+
diff --git a/LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load-expected.txt b/LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load-expected.txt
new file mode 100644 (file)
index 0000000..64e2ca5
--- /dev/null
@@ -0,0 +1,3 @@
+
+[1]
+SUCCESS
diff --git a/LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load.html b/LayoutTests/fast/block/positioning/positioned-float-layout-after-image-load.html
new file mode 100644 (file)
index 0000000..4263e18
--- /dev/null
@@ -0,0 +1,21 @@
+<html>
+<head>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+     }
+
+    //   https://bugs.webkit.org/show_bug.cgi?id=54611
+    //   When navigating to positioned-float-layout-after-image-load-2.html#anchor1 directly, the anchor should be positioned below 
+    //   the image on first load. The test is sensitive to caching of the image, so you should reload 
+    //   positioned-float-layout-after-image-load-2.html#anchor1 if testing manually.
+
+    function test(){
+        setTimeout(location.assign("resources/positioned-float-layout-after-image-load-2.html#anchor1"),0);
+    }
+</script>
+</head>
+<body onload="test();">
+</body></html>
+
diff --git a/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html b/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html
new file mode 100644 (file)
index 0000000..e5d8f82
--- /dev/null
@@ -0,0 +1,37 @@
+<html>
+<head>
+<style type="text/css">
+  body { margin-left: 10%; margin-right: 10%;}
+  h1,h2,h3,h4,h5,h6 { text-align: center; clear: both;}
+  h1 span { display: block; padding-bottom: 0.5em; }
+  .figcenter   { margin: auto; text-align: center;}
+  .footnotes        {border: dashed 1px;}
+  .footnote         {margin-left: 10%; margin-right: 10%; font-size: 0.9em;}
+  .footnote .label  {position: absolute; right: 84%; text-align: right;}
+</style>
+
+<script>
+    //   https://bugs.webkit.org/show_bug.cgi?id=54611
+    //   When navigating to the anchor directly, the anchor should be positioned below 
+    //   the image on first load.
+
+    function test(){
+        var bottomOfImage = document.getElementById("image").offsetTop + document.getElementById("image").offsetHeight;
+        var footnotePosition = document.getElementById("spantext").offsetTop;
+        if (footnotePosition >= bottomOfImage)
+          document.getElementById("console").innerHTML = "SUCCESS";
+        layoutTestController.notifyDone();
+    }
+</script>
+
+</head>
+<body onload="test();">
+<div class="figcenter">
+  <img id="image" src="icon-gold.png" alt="Book cover." title="">
+</div>
+
+<div class="footnote"><a id="anchor1"></a><a href="http://www.gutenberg.org/files/33439/33439-h/33439-h.htm#FNanchor_1"><span id="spantext" class="label">[1]</span></a></div>
+
+<div id="console">FAILURE</div>
+</body></html>
+
diff --git a/LayoutTests/fast/block/positioning/resources/positioned-float-layout-after-image-load-2.html b/LayoutTests/fast/block/positioning/resources/positioned-float-layout-after-image-load-2.html
new file mode 100644 (file)
index 0000000..21f150c
--- /dev/null
@@ -0,0 +1,36 @@
+<html>
+<head>
+<style type="text/css">
+  body { margin-left: 10%; margin-right: 10%;}
+  h1,h2,h3,h4,h5,h6 { text-align: center; clear: both;}
+  h1 span { display: block; padding-bottom: 0.5em; }
+  .figcenter   { margin: auto; text-align: center;}
+  .footnote         {margin-left: 10%; margin-right: 10%; font-size: 0.9em;}
+  .footnote .label  {position: absolute; float: -webkit-positioned; right: 84%; text-align: right; width: 200px}
+</style>
+
+<script>
+    //   https://bugs.webkit.org/show_bug.cgi?id=54611
+    //   When navigating to the anchor directly, the anchor should be positioned below 
+    //   the image on first load.
+
+    function test(){
+        var bottomOfImage = document.getElementById("image").offsetTop + document.getElementById("image").offsetHeight;
+        var footnotePosition = document.getElementById("spantext").offsetTop;
+        if (footnotePosition >= bottomOfImage)
+          document.getElementById("console").innerHTML = "SUCCESS";
+        layoutTestController.notifyDone();
+    }
+</script>
+
+</head>
+<body onload="test();">
+<div class="figcenter">
+  <img id="image" src="icon-gold.png" alt="Book cover." title="">
+</div>
+
+<div class="footnote"><a id="anchor1"></a><a href="http://www.gutenberg.org/files/33439/33439-h/33439-h.htm#FNanchor_1"><span id="spantext" class="label">[1]</span></a></div>
+
+<div id="console">FAILURE</div>
+</body></html>
+
index 8ff3fc4bca0b5872695682eacdc2c275845363fa..318dfd24fd2612f48804b06dd1824a65f47b38b7 100644 (file)
@@ -1,3 +1,32 @@
+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
+              fast/block/positioning/positioned-float-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. Do the same
+        for positioned floats in RenderBlock::positionedFloatsNeedRelayout(), although currently layoutPositionedObjects()
+        takes care of it this at least ensures no regression in future.
+
+        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.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::layoutPositionedObjects): remove the check for r->parent()->isBlockFlow() when 
+                                                         deciding whether to mark children for layout
+        (WebCore::RenderBlock::positionedFloatsNeedRelayout): ditto
+
 2011-09-07  Anna Cavender  <annacc@chromium.org>
 
         Moving platform/track to html/track to avoid layering violation.
index f40ad684a5b3ab3c927b56e366c06595886f85fc..840330ef945920e4e9b4b4919ef29a56c028a9e7 100644 (file)
@@ -2216,7 +2216,7 @@ bool RenderBlock::positionedFloatsNeedRelayout()
         if (positionedObject->needsLayout())
             return true;
 
-        if (positionedObject->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && positionedObject->parent() != this && positionedObject->parent()->isBlockFlow())
+        if (positionedObject->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && positionedObject->parent() != this)
             return true;
         
         if (view()->layoutState()->pageLogicalHeightChanged() || (view()->layoutState()->pageLogicalHeight() && view()->layoutState()->pageLogicalOffset(logicalTop()) != pageLogicalOffset()))
@@ -2244,7 +2244,7 @@ bool RenderBlock::layoutPositionedObjects(bool relayoutChildren)
         // non-positioned block.  Rather than trying to detect all of these movement cases, we just always lay out positioned
         // 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);
             
         // If relayoutChildren is set and the child has percentage padding or an embedded content box, we also need to invalidate the childs pref widths.