Elements with position:absolute don't move to correct position after images load

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Gareth Rees]

Bug seen in Chrome 9.0.597.102, Safari 5.0.3, and iCab 4.8.0.

The page <http://www.gutenberg.org/files/33439/33439-h/33439-h.htm#Footnote_1> has a list of footnotes with labels that are placed to the left of each footnote using this style:

.footnote .label  {position: absolute; right: 84%; text-align: right;}

When visiting the page, the footnote labels start out placed correctly (next to their corresponding footnote), but then as the images load, the footnotes get pushed downwards while the labels stay put, making it impossible to tell which label belongs to which footnote.

If the page is made to redo the layout after the images have finished loading (for example, by adjusting the width of the browser window), the footnote labels return to the correct position.

This could be worked around in the page source by specifying image sizes, or by using a different technique to position the footnote labels, but this behaviour looks may be a bug in WebKit. In Opera 11.01 and Firefox 3.6.10 the footnotes remain in their correct positions while the images load.

[Patrick R. Gansterer]

Reproduced with r79118.

Can you provide a (more) reduced test case?

[Robert Hogan]

The reduced case for this is:

<style type="text/css">
body { margin-left: 10%; margin-right: 10%;}
.footnote         {margin-left: 10%; margin-right: 10%; font-size: 0.9em;}
.footnote .label  {position: absolute; right: 84%; text-align: right;}
</style>
<img src="http://www.gutenberg.org/files/33439/33439-h/images/cover.jpg">
<div class="footnote"><a id="Footnote_1"></a><a href="resources/greenbox.png"><div class="label">[1]</div></a></div>

The 'label' block is an absolutely positioned child of an inline flow. So during layout, this line from RenderBlock::layoutPositionedObjects fails to dirty it for rendering:

        if (relayoutChildren || (r->style()->hasStaticBlockPosition(isHorizontalWritingMode()) && r->parent() != this && r->parent()->isBlockFlow()))
            r->setChildNeedsLayout(true, false);

The culprit is r->parent()->isBlockFlow(). Thie code was introduced in http://trac.webkit.org/changeset/8284. It's not clear to me why the requirement that the parent is a block flow is important here, other than to assume valid HTML. 

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.

Dave/Simon, is there a good reason for expecting the parent to be a block flow?

[Robert Hogan]

Created attachment 100206 [details]
Test Case

[Robert Hogan]

Created attachment 106261 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 106261 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106261&action=review

> LayoutTests/ChangeLog:10
> +        * http/tests/navigation/anchor-rendered-after-image-load-expected.txt: Added.
> +        * http/tests/navigation/anchor-rendered-after-image-load.html: Added.
> +        * http/tests/navigation/resources/anchor-rendered-after-image-load.html: Added.

I don't understand why this has to be an http test.

[Robert Hogan]

(In reply to comment #5)
> 
> I don't understand why this has to be an http test.

The issue is encountered only on first load without a fragment identifier, it happens reliably when you include the fragment identifier in the url. (See comment #2). So I wrote the test so that it would fail reliably if there was a regression - loading the page with the fragment identifier. It was easier to do this in http/tests.

[Simon Fraser (smfr)]

But you should be able to make a normal layout test with a little bit of JS that reproduces the same issue, no?

[Robert Hogan]

Created attachment 106285 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 106285 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106285&action=review

> LayoutTests/fast/block/positioning/absolute-layout-after-image-load.html:3
> +<meta http-equiv="Content-Style-Type" content="text/css">

Remove

> LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html:3
> +<meta http-equiv="Content-Style-Type" content="text/css">

Remove.

> LayoutTests/fast/block/positioning/resources/absolute-layout-after-image-load-2.html:17
> +        layoutTestController.waitUntilDone();

I don't think you need these in the loaded resource. Those in the first test page are enough.

> Source/WebCore/ChangeLog:16
> +        The reduced case for this bug is:
> +
> +        <style type="text/css">
> +        .footnote .label  {position: absolute; right: 84%; text-align: right;}
> +        </style>
> +        <img src="http://www.gutenberg.org/files/33439/33439-h/images/cover.jpg">
> +        <div class="footnote"><a id="Footnote_1"></a><a href="resources/greenbox.png"><div class="label">[1]</div></a></div>

It's not necessary to put this in the changelog.

[Robert Hogan]

Committed r94695: <http://trac.webkit.org/changeset/94695>

[Ryosuke Niwa]

The test added by this patch is hitting an assertion on Snow Leopard. I've filed https://bugs.webkit.org/show_bug.cgi?id=67759 to track this failure.

[Zoltan Horvath]

I rolled out the patch (http://trac.webkit.org/changeset/94755) because it hits assertion on Snow Leopard, Qt, GTK.

[Robert Hogan]

Committed r94785: <http://trac.webkit.org/changeset/94785>