Bug 114879

Summary:

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

Product:

WebKit

Reporter:

Boris Zbarsky <bzbarsky>

Component:

Layout and Rendering

Assignee:

Robert Hogan <robert>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, esprehn+autocc, glenn, hyatt, rniwa, robert

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

117824

Bug Blocks:

Attachments:

Description	Flags
Testcase	none
Patch	none
Patch	none
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion	none
Patch	none
Patch	none

Boris Zbarsky

Reported 2013-04-19 09:56:54 PDT

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...

Attachments
Testcase (322 bytes, text/html) 2013-04-19 09:56 PDT, Boris Zbarsky	no flags	Details
Patch (5.46 KB, patch) 2013-05-06 14:25 PDT, Robert Hogan	no flags	Details Formatted Diff Diff
Patch (22.07 KB, patch) 2013-05-27 13:01 PDT, Robert Hogan	no flags	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (1.33 MB, application/zip) 2013-05-27 16:33 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (4.95 MB, application/zip) 2013-05-29 00:28 PDT, Build Bot	no flags	Details
Patch (46.40 KB, patch) 2013-06-17 12:15 PDT, Robert Hogan	no flags	Details Formatted Diff Diff
Patch (46.40 KB, patch) 2013-06-17 12:24 PDT, Robert Hogan	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Robert Hogan

Comment 1 2013-05-06 14:25:31 PDT

Created attachment 200814 [details] Patch

Dave Hyatt

Comment 2 2013-05-20 12:49:24 PDT

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

Comment 3 2013-05-20 13:11:51 PDT

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

Comment 4 2013-05-20 13:38:43 PDT

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

Comment 5 2013-05-27 13:01:54 PDT

Created attachment 203005 [details] Patch

Build Bot

Comment 6 2013-05-27 16:33:04 PDT

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

Comment 7 2013-05-27 16:33:05 PDT

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 8 2013-05-29 00:28:27 PDT

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

Comment 9 2013-05-29 00:28:29 PDT

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

Comment 10 2013-06-17 12:15:10 PDT

Created attachment 204847 [details] Patch

Robert Hogan

Comment 11 2013-06-17 12:24:00 PDT

Created attachment 204848 [details] Patch

Dave Hyatt

Comment 12 2013-06-18 13:05:20 PDT

Comment on attachment 204848 [details] Patch Looks good. Glad to see that line go, since it's bogus.

Dave Hyatt

Comment 13 2013-06-18 13:21:11 PDT

Comment on attachment 204848 [details] Patch r=me