Bug 114879 – Incorrect computation of shrink-to-fit width for block with white-space:nowrap and floating children

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Boris Zbarsky]

Created attachment 198894 [details]
Testcase

Consider the attached testcase.

The width of the first green box should be the minimum width as defined at http://www.w3.org/TR/CSS21/visudet.html#shrink-to-fit-float which means it should be the width if all allowed line-breaks are taken.

The second green box shows that a line-break between the two floats is in fact allowed, if it's needed.

So the first green box should end up with a width of 16px and the two floats in it should stack on top of each other.  But in WebKit they end up next to each other.

Gecko, Trident, Presto get this right as far as I can tell.

Note that removing the "white-space:nowrap" makes WebKit behave correctly, which is odd, since white-space does not affect float positioning...

[Robert Hogan]

Created attachment 200814 [details]
Patch

[Dave Hyatt]

Comment on attachment 200814 [details]
Patch

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

r-

> Source/WebCore/rendering/RenderBlock.cpp:5812
> +    bool shrinkToFit = (!isReplaced() && isFloatingOrOutOfFlowPositioned()) || (isInlineBlockOrInlineTable() && !isTable());
> +    if (!style()->autoWrap() && !shrinkToFit && childrenInline()) {

Why special case shrinkToFit? If I'm understanding the bug correctly, it's that we should ignore nowrap completely when computing min intrinsic sizes. Anyway, you'd want to use something like:

RenderBox::sizesLogicalWidthToFitContent

to really catch all the shrink to fit cases, but I'm not convinced that it's correct to do so. It sounds like minimum width is supposed to ignore white-space if I'm understanding this correctly. You'll want to double check and see if tables behave properly if we make that change though.

[Boris Zbarsky]

nowrap is most certainly supposed to affect intrinsic sizes.

What it's not supposed to affect is whether floats can clear past each other and the effect of such floats on intrinsic sizes.

[Dave Hyatt]

This is a float bug. You are always allowed to break before or after a float, so need to just patch the code to handle that. It looks like the code sort of incorrectly tried to handle it but then forgot to set inlineMin to 0.

[Robert Hogan]

Created attachment 203005 [details]
Patch

[Build Bot]

Comment on attachment 203005 [details]
Patch

Attachment 203005 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/749008

New failing tests:
fast/replaced/width100percent-image.html
fast/replaced/width100percent-textarea.html
tables/mozilla/bugs/bug57828.html
fast/replaced/width100percent-textfield.html
fast/replaced/width100percent-menulist.html
fast/css/word-space-extra.html
fast/replaced/width100percent-searchfield.html

[Build Bot]

Created attachment 203016 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 203005 [details]
Patch

Attachment 203005 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/693239

New failing tests:
fast/replaced/width100percent-image.html
fast/replaced/width100percent-textarea.html
tables/mozilla/bugs/bug57828.html
fast/replaced/width100percent-textfield.html
fast/replaced/width100percent-menulist.html
fast/css/word-space-extra.html
fast/replaced/width100percent-searchfield.html

[Build Bot]

Created attachment 203130 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Robert Hogan]

Created attachment 204847 [details]
Patch

[Robert Hogan]

Created attachment 204848 [details]
Patch

[Dave Hyatt]

Comment on attachment 204848 [details]
Patch

Looks good. Glad to see that line go, since it's bogus.

[Dave Hyatt]

Comment on attachment 204848 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 204848 [details]
Patch

Clearing flags on attachment: 204848

Committed r151737: <http://trac.webkit.org/changeset/151737>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.