RESOLVED CONFIGURATION CHANGED 37245
Don't break before positioned objects while skipping through trailing space
https://bugs.webkit.org/show_bug.cgi?id=37245
Summary Don't break before positioned objects while skipping through trailing space
Andy Estes
Reported 2010-04-07 19:35:44 PDT
The 'Sign In' button is missing from the 'Manage My Direct Loan' box on the front page of http://studentloans.gov. The button is visible in Firefox 3.6.3. Have not tested on Windows.
Attachments
Screenshot from Firefox (296.27 KB, image/png)
2010-04-07 19:40 PDT, Andy Estes
no flags
Screenshot from Safari (276.29 KB, image/png)
2010-04-07 19:41 PDT, Andy Estes
no flags
StudentLoans test (7.50 KB, application/octet-stream)
2010-04-12 18:56 PDT, Nate Whetsell
no flags
Simple example with whitespace (246 bytes, text/html)
2010-04-12 18:58 PDT, Nate Whetsell
no flags
Simple example without whitespace (245 bytes, text/html)
2010-04-12 18:59 PDT, Nate Whetsell
no flags
Patch (4.88 KB, patch)
2013-04-17 12:27 PDT, Robert Hogan
no flags
Patch (5.59 KB, patch)
2013-04-18 11:14 PDT, Robert Hogan
no flags
Patch (5.59 KB, patch)
2013-04-20 04:05 PDT, Robert Hogan
no flags
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (515.59 KB, application/zip)
2013-04-20 05:52 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (501.04 KB, application/zip)
2013-04-20 07:13 PDT, Build Bot
no flags
Patch (5.66 KB, patch)
2013-04-21 01:22 PDT, Robert Hogan
no flags
Patch (7.75 KB, patch)
2013-04-22 12:44 PDT, Robert Hogan
no flags
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (491.34 KB, application/zip)
2013-04-22 13:56 PDT, Build Bot
no flags
Patch (8.88 KB, patch)
2013-04-22 14:42 PDT, Robert Hogan
no flags
Patch (8.09 KB, patch)
2013-04-25 12:06 PDT, Robert Hogan
no flags
Patch (7.77 KB, patch)
2013-05-01 14:12 PDT, Robert Hogan
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (631.05 KB, application/zip)
2013-05-01 16:00 PDT, Build Bot
no flags
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (607.85 KB, application/zip)
2013-05-01 18:18 PDT, Build Bot
no flags
Andy Estes
Comment 1 2010-04-07 19:40:36 PDT
Created attachment 52821 [details] Screenshot from Firefox Firefox 3.6.3 on Mac OS X 10.6.3.
Andy Estes
Comment 2 2010-04-07 19:41:18 PDT
Created attachment 52822 [details] Screenshot from Safari Safari 4.0.5 on Mac OS X 10.6.3.
Andy Estes
Comment 3 2010-04-07 19:44:01 PDT
Andy Estes
Comment 4 2010-04-07 19:53:17 PDT
Reproduces in Safari 4.0.5 on Windows as well.
Nate Whetsell
Comment 5 2010-04-12 18:53:16 PDT
The issue appears to be due to differences between how Firefox (Gecko) and Safari (WebKit) do layouts when position:absolute is used, but top/bottom/left/right are not. In these cases, top/bottom/left/right are all set to "auto," so the browser is free to position such elements wherever it wants to. Firefox seems to place the element *to the right* of whatever came before it. Safari seems to place the element *below* whatever came before it (sometimes -- more on this later). StudentLoans.gov uses absolute positioning, but uses margins instead of top/bottom/left/right to set actual positions. The negative margin used to place the sign-in button is roughly equal to the width of the "main image" (with all the smiling people in it) plus the width of the "Manage My Direct Loan" image. In Firefox, this will currently work because the sign-in button is actually placed to the right of the main image. In Safari, because the sign-in button is on the line after the main image, moving it so far to the right moves it out of the window. I will attach a simplified example called "StudentLoans test" illustrating this. I'll need to do this as a ZIP file because of the necessary images. What's odd, though, is that Safari's automatic placement behavior is variable. I will attach two more examples (HTML only) illustrating this. The presence of whitespace between text and an absolutely positioned element affects automatic placement.
Nate Whetsell
Comment 6 2010-04-12 18:56:27 PDT
Created attachment 53210 [details] StudentLoans test
Nate Whetsell
Comment 7 2010-04-12 18:58:58 PDT
Created attachment 53211 [details] Simple example with whitespace
Nate Whetsell
Comment 8 2010-04-12 18:59:36 PDT
Created attachment 53212 [details] Simple example without whitespace
Robert Hogan
Comment 9 2013-04-16 10:08:32 PDT
This is a dupe of bug 45274 to all intents and purposes. Can confirm once it lands.
Robert Hogan
Comment 10 2013-04-17 12:27:54 PDT
Robert Hogan
Comment 11 2013-04-18 11:14:47 PDT
Benjamin Poulain
Comment 12 2013-04-18 14:17:32 PDT
Comment on attachment 198749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198749&action=review > LayoutTests/fast/inline/trailing-space-abspos-span.html:13 > + <!-- webkit.org/b/37245: The xxx and yyy should be all on the same line --> In my opinion, it is good to have a complete explanation in the test, and have it in the markup instead of in comments. When a test is failing, it makes it easier to just open it in Safari and directly see what is expected and how it is failing.
Robert Hogan
Comment 13 2013-04-18 14:57:30 PDT
(In reply to comment #12) > (From update of attachment 198749 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=198749&action=review > > > LayoutTests/fast/inline/trailing-space-abspos-span.html:13 > > + <!-- webkit.org/b/37245: The xxx and yyy should be all on the same line --> > > In my opinion, it is good to have a complete explanation in the test, and have it in the markup instead of in comments. > > When a test is failing, it makes it easier to just open it in Safari and directly see what is expected and how it is failing. Me too. I'm testing the y-position of the line though and this would be affected by the height of the text above the line - which would differ from platform to platform. Could probably use line-height to fix that but didn't think too deeply and left it as a comment instead.
Robert Hogan
Comment 14 2013-04-20 04:05:38 PDT
Build Bot
Comment 15 2013-04-20 05:52:04 PDT
Comment on attachment 198939 [details] Patch Attachment 198939 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/159368 New failing tests: fast/inline/trailing-space-abspos-span.html
Build Bot
Comment 16 2013-04-20 05:52:06 PDT
Created attachment 198941 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Build Bot
Comment 17 2013-04-20 07:13:56 PDT
Comment on attachment 198939 [details] Patch Attachment 198939 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/72860 New failing tests: fast/inline/trailing-space-abspos-span.html
Build Bot
Comment 18 2013-04-20 07:13:58 PDT
Created attachment 198942 [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.2
Robert Hogan
Comment 19 2013-04-21 01:22:26 PDT
Dave Hyatt
Comment 20 2013-04-22 11:48:33 PDT
Comment on attachment 198959 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=198959&action=review r- > Source/WebCore/rendering/RenderBlockLineLayout.cpp:3259 > + if (checkForBreak && !width.fitsOnLine(m_block->isFloatingOrOutOfFlowPositioned())) { Shouldn't it be all shrink to fit and not just floating or positioned? What about say inline blocks? > Source/WebCore/rendering/RenderBlockLineLayout.cpp:3297 > + if (width.fitsOnLine(m_block->isFloatingOrOutOfFlowPositioned()) || lastWS == NOWRAP) Same.
Robert Hogan
Comment 21 2013-04-22 12:44:26 PDT
Build Bot
Comment 22 2013-04-22 13:56:51 PDT
Comment on attachment 199091 [details] Patch Attachment 199091 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/16487 New failing tests: fast/inline/trailing-space-shrink-to-fit-parent.html fast/css-intrinsic-dimensions/css-tables.html fast/css-intrinsic-dimensions/tables.html
Build Bot
Comment 23 2013-04-22 13:56:54 PDT
Created attachment 199103 [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.2
Robert Hogan
Comment 24 2013-04-22 14:42:31 PDT
Dave Hyatt
Comment 25 2013-04-23 11:30:12 PDT
Comment on attachment 199111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=199111&action=review > Source/WebCore/rendering/RenderBlockLineLayout.cpp:2742 > +static bool shrinksToFit(RenderObject* renderer) > +{ > + if (renderer->isFloatingOrOutOfFlowPositioned()) > + return true; > + > + EDisplay display = renderer->style()->display(); > + return display == INLINE_BLOCK || display == INLINE_TABLE || display == TABLE_CELL; > +} I am not understanding this patch at all. I don't get why shrink-to-fit should result in special behavior here. shrink-to-fit can be caused by any number of things, from a specified intrinsic width in CSS to default behavior for certain objects. That width can further be influenced by max/min-width to no longer be intrinsic, or maybe the container has a specified fixed width, etc. I need to understand more about what is going on here in order to understand why a behavioral change is needed only under certain circumstances. I'm unconvinced that shrink-to-fit is the right thing to be testing here.
Robert Hogan
Comment 26 2013-04-25 12:06:09 PDT
Robert Hogan
Comment 27 2013-05-01 14:12:58 PDT
Build Bot
Comment 28 2013-05-01 16:00:22 PDT
Comment on attachment 200239 [details] Patch Attachment 200239 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/378117 New failing tests: fast/canvas/webgl/webgl-layer-update.html fast/block/positioning/relative-positioned-inline-container.html fast/forms/onselect-textarea.html fast/canvas/webgl/webgl-composite-modes.html media/audio-controls-rendering.html compositing/iframes/composited-iframe-alignment.html fast/forms/onselect-textfield.html fast/canvas/webgl/webgl-composite-modes-repaint.html
Build Bot
Comment 29 2013-05-01 16:00:25 PDT
Created attachment 200248 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Build Bot
Comment 30 2013-05-01 18:18:43 PDT
Comment on attachment 200239 [details] Patch Attachment 200239 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/380144 New failing tests: fast/canvas/webgl/webgl-layer-update.html fast/block/positioning/relative-positioned-inline-container.html fast/forms/onselect-textarea.html fast/canvas/webgl/webgl-composite-modes.html media/audio-controls-rendering.html compositing/iframes/composited-iframe-alignment.html fast/forms/onselect-textfield.html fast/canvas/webgl/webgl-composite-modes-repaint.html
Build Bot
Comment 31 2013-05-01 18:18:45 PDT
Created attachment 200294 [details] Archive of layout-test-results from webkit-ews-06 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.2
Ahmad Saleem
Comment 32 2022-05-30 13:09:22 PDT
I am able to reproduce the issue from the test case (with whitespace) in Safari 15.5. While Chrome Canary 104 and Firefox Nightly 102 renders this similar to (without whitespace) output of Safari 15.5 on macOS 12.4. Thanks!
Ahmad Saleem
Comment 33 2023-01-12 16:30:39 PST
(In reply to Ahmad Saleem from comment #32) > I am able to reproduce the issue from the test case (with whitespace) in > Safari 15.5. While Chrome Canary 104 and Firefox Nightly 102 renders this > similar to (without whitespace) output of Safari 15.5 on macOS 12.4. Thanks! It seems that the failing (with whitespace) test case is also passing in STP161. @Alan - do we need to do anything else? Thanks!
zalan
Comment 34 2023-01-12 16:45:33 PST
(In reply to Ahmad Saleem from comment #33) > (In reply to Ahmad Saleem from comment #32) > > I am able to reproduce the issue from the test case (with whitespace) in > > Safari 15.5. While Chrome Canary 104 and Firefox Nightly 102 renders this > > similar to (without whitespace) output of Safari 15.5 on macOS 12.4. Thanks! > > It seems that the failing (with whitespace) test case is also passing in > STP161. > > @Alan - do we need to do anything else? Thanks! Thank you for testing, Ahmad! (this is an IFC progression)
Note You need to log in before you can comment on or make changes to this bug.