Bug 237732

Summary: [css-grid] Grid contents sometimes layout with the wrong width
Product: WebKit Reporter: Matt Woodrow <mattwoodrow>
Component: Layout and RenderingAssignee: Matt Woodrow <mattwoodrow>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, obrufau, pdr, rego, sebastian.sams, simon.fraser, svillar, webkit-bug-importer, zalan, zsun
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://www.economist.com/the-economist-explains/2022/02/03/how-apples-privacy-push-cost-meta-10bn
See Also: https://bugs.webkit.org/show_bug.cgi?id=209282
Attachments:
Description Flags
Patch
none
Patch none

Matt Woodrow
Reported 2022-03-10 13:46:38 PST
<rdar://88512506> Resizing the browser on the provided URL sometimes lays out the main article text to the wrong width, where it doesn't line wrap at all and just extends off the right side of the page. It looks like we call RenderGrid::computeIntrinsicLogicalWidths and that results in us calling layoutIfNeeded on the child RenderBlock with an estimated width (which turns out to be wrong). Now that we've mutated the child and laid it out incorrectly, nothing marks the RenderGrid itself as needing layout, so we never do a proper layout with the final grid column sizes to fix this again. The bug happens intermittently, since anytime we do a full layout of the grid, we also layout the children correctly.
Attachments
Patch (6.65 KB, patch)
2022-03-16 20:08 PDT, Matt Woodrow 		no flags
DetailsFormatted DiffDiff
Patch (13.45 KB, patch)
2022-03-28 17:02 PDT, Matt Woodrow 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Matt Woodrow
Comment 1 2022-03-13 14:16:16 PDT
*** Bug 237710 has been marked as a duplicate of this bug. ***
Matt Woodrow
Comment 2 2022-03-13 14:17:25 PDT
Bug 237710 has a reduced test case for this: https://bugs.webkit.org/attachment.cgi?id=454343 This is a regression from https://bugs.webkit.org/show_bug.cgi?id=232140
Matt Woodrow
Comment 3 2022-03-13 14:32:28 PDT
It seems like we shouldn't be laying out children of a RenderGrid from (const!) computeIntrinsicLogicalWidths using estimated grid breadths. Could we cache the intrinsic sizes when we do a full layout, and reuse those if we haven't marked the RenderGrid as needing layout? A quick prototype of that seems to work.
Matt Woodrow
Comment 4 2022-03-13 18:53:49 PDT
Looks like this showed up previously, but went away due to a change in the flex code. This is new variant of the same existing issue. https://bugs.webkit.org/show_bug.cgi?id=209282
Matt Woodrow
Comment 5 2022-03-16 20:08:42 PDT
Created attachment 454926 [details] Patch
Javier Fernandez
Comment 6 2022-03-17 15:39:40 PDT
Comment on attachment 454926 [details] Patch The change looks fine, but there are a few tests that regressed.
Matt Woodrow
Comment 7 2022-03-28 17:02:38 PDT
Created attachment 455971 [details] Patch
EWS
Comment 8 2022-03-29 16:05:31 PDT
Committed r292079 (249006@main): <https://commits.webkit.org/249006@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455971 [details].
Note You need to log in before you can comment on or make changes to this bug.