- Revision
- 94785
- Author
- [email protected]
- Date
- 2011-09-08 12:58:32 -0700 (Thu, 08 Sep 2011)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (94784 => 94785)
--- trunk/LayoutTests/ChangeLog 2011-09-08 19:56:38 UTC (rev 94784)
+++ trunk/LayoutTests/ChangeLog 2011-09-08 19:58:32 UTC (rev 94785)
@@ -1,3 +1,14 @@
+2011-09-03 Robert Hogan <[email protected]>
+
+ 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 <[email protected]>
<style scoped>: Add 'scoped' attribute
Added: trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt (0 => 94785)
--- trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt 2011-09-08 19:58:32 UTC (rev 94785)
@@ -0,0 +1,3 @@
+
+[1]
+SUCCESS
Property changes on: trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html (0 => 94785)
--- trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html (rev 0)
+++ trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html 2011-09-08 19:58:32 UTC (rev 94785)
@@ -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>
+
Property changes on: trunk/LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html (0 => 94785)
--- trunk/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html (rev 0)
+++ trunk/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html 2011-09-08 19:58:32 UTC (rev 94785)
@@ -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="" alt="Book cover." title="">
+</div>
+
+<div class="footnote"><a id="anchor1"></a><a href="" id="spantext" class="label">[1]</span></a></div>
+
+<div id="console">FAILURE</div>
+</body></html>
+
Property changes on: trunk/LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (94784 => 94785)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-09-08 19:56:38 UTC (rev 94784)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-09-08 19:58:32 UTC (rev 94785)
@@ -3714,5 +3714,3 @@
BUG_REED : fast/canvas/canvas-text-baseline.html = IMAGE
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
Modified: trunk/Source/WebCore/ChangeLog (94784 => 94785)
--- trunk/Source/WebCore/ChangeLog 2011-09-08 19:56:38 UTC (rev 94784)
+++ trunk/Source/WebCore/ChangeLog 2011-09-08 19:58:32 UTC (rev 94785)
@@ -1,3 +1,32 @@
+2011-09-03 Robert Hogan <[email protected]>
+
+ 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 <[email protected]>
<style scoped>: Add 'scoped' attribute
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (94784 => 94785)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2011-09-08 19:56:38 UTC (rev 94784)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2011-09-08 19:58:32 UTC (rev 94785)
@@ -2244,7 +2244,7 @@
// 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.
