Bug 103116 – REGRESSION(r127163): Content is offset to the right at rea.ru

• Component: Layout and Rendering
• Status: RESOLVED FIXED

Created attachment 175768 [details]
Screenshot of incorrect rendering in Chrome 23 and correct rendering in IE 9

Incorrect rendering at www.rea.ru. Right panel is showed under left and center panels, and left and center panels are moved to the right.
It renders correctly if the size of the browser window is less than ~1100px or if you zoom out to <90%.

[Daniel Bates]

Created attachment 175838 [details]
WebArchive for rea.ru

For historical preservation, a web archive of <http://www.rea.ru> as it appeared on 11/23/2012.

[Daniel Bates]

Created attachment 175839 [details]
HTML source for rea.ru

The HTML source code for <http://www.rea.ru> as accessed on 11/23/2012. I added a <meta> content-type tag so that this file can be rendered offline with respect to the correct character set. For completeness, www.rea.ru returned the content-type and character encoding information is its HTTP response headers.

[Daniel Bates]

Confirmed that Mac Nightly r135516 renders <http://www.rea.ru> analogous to the Chrome 23 screen included in the attached screenshot, attachment #175768 [details].  Both shipping Safari version 6.0.2 (8536.26.17) and Mac Nightly r127143 (the latest nightly less than r127151, since r127151 isn't available as a nightly) render the site analogous to the IE 9 screen in the attached screenshot. And the IE 9 screen is consistent with how the site appears in Firefox for Mac 16.0.2 and Opera for Mac 12.10.

[Alexey Proskuryakov]

Bisected to <http://trac.webkit.org/changeset/127163>.

[Alexey Proskuryakov]

<rdar://problem/12745458>

[Robert Hogan]

Created attachment 175878 [details]
Reduction

[Robert Hogan]

I think the new rendering is correct. 

<div style="height: 20px; width:20px; float:left; background: blue;"></div>
<div style="margin-top:10px; clear:both;">
    <div style="height: 20px; width:20px; float:left; background: black;"></div>
</div>

The div with margin-top is self-collapsing, so calculates its clearance using the  rule:  [height of float to clear] - margin-top of the
self-collapsing block. (See the 'Explanation' section near the end of ​http://www.w3.org/TR/CSS21/visuren.html#clearance). 

So the black float inside the self-collapsing block avoids the blue float but doesn't move below it as the calculated clearance for its parent is only 10px.

[Robert Hogan]

This comes down to the correct 'top' position for the black float in the reduction. 

In our current rendering we calculate negative clearance for the black float's container, so its outer top starts halfway down the blue float and its border box starts at the bottom of the blue float. This is per http://www.w3.org/TR/CSS21/visuren.html#clearance

So what we need is the spec to tell us that the outer top of the black float should be the same as the border top of its container rather than its margin top. But I don't think it does say that. 

The relevant parts of the spec are:

http://www.w3.org/TR/CSS21/visuren.html#propdef-float

"4. A floating box's outer top may not be higher than the top of its containing block. When the float occurs between two collapsing margins, the float is positioned as if it had an otherwise empty anonymous block parent taking part in the flow. The position of such a parent is defined by the rules in the section on margin collapsing."

http://www.w3.org/TR/CSS21/box.html#collapsed-through

"If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it. In this case, the position of the element depends on its relationship with the other elements whose margins are being collapsed.

- If the element's margins are collapsed with its parent's top margin, the top border edge of the box is defined to be the same as the parent's.

- Otherwise, either the element's parent is not taking part in the margin collapsing, or only the parent's bottom margin is involved. The position of the element's top border edge is the same as it would have been if the element had a non-zero bottom border.

Note that the positions of elements that have been collapsed through have no effect on the positions of the other elements with whose margins they are being collapsed; the top border edge position is only required for laying out descendants of these elements."

[Eric Seidel (no email)]

It seems that regardless of what the spec says (or fails to say), we're the odd-browser out here. :)  Everyone else renders this test case "correctly".

[Robert Hogan]

(In reply to comment #9)
> It seems that regardless of what the spec says (or fails to say), we're the odd-browser out here. :)  Everyone else renders this test case "correctly".

Asked at www-style: http://lists.w3.org/Archives/Public/www-style/2012Nov/0418.html

[Robert Hogan]

Created attachment 176704 [details]
Patch

[Robert Hogan]

Created attachment 177310 [details]
Patch

[Dave Hyatt]

Comment on attachment 177310 [details]
Patch

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

r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2134
> +            m_block->setLogicalHeight(m_block->logicalHeight() - marginOffset);

This is a very roundabout way of just setting the block back to its previous logical height. I would prefer it if you just stored the block's logical height in an oldLogicalHeight variable and set the block's height back to that variable. I think that's more readable than this even though it adds one more line of code.

[Robert Hogan]

Created attachment 178039 [details]
Patch

[Dave Hyatt]

Comment on attachment 178039 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 178039 [details]
Patch

Clearing flags on attachment: 178039

Committed r136967: <http://trac.webkit.org/changeset/136967>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Even though the reduction shows this to be fixed, the webarchive still shows the main content being offset (the live site is OK).

The site has a workaround now. If you remove it, it still looks like it was before fixing this bug. Remove the following style from the top of the page to check that:

#ctl00_ContentPlaceHolder1_DESPAGE div div div
{
   overflow:hidden;
}