Bug 163434

Summary: [css-grid] Changing the argument on fit-content() doesn't cause the grid to be relayout
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: CSSAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, jfernandez, kondapallykalyan, rego, svillar
Priority: P2 Keywords: BlinkMergeCandidate
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=654712
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Manuel Rego Casasnovas
Reported 2016-10-14 01:39:34 PDT
If you modify the argument of a fit-content() track from JavaScript, the change doesn't get applied. This has been already fixed on Blink: https://codereview.chromium.org/2420673002
Attachments
Patch (5.87 KB, patch)
2016-10-14 01:42 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch (5.82 KB, patch)
2016-10-14 02:07 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.92 KB, patch)
2016-10-14 08:20 PDT, Manuel Rego Casasnovas 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Manuel Rego Casasnovas
Comment 1 2016-10-14 01:42:32 PDT
Created attachment 291593 [details] Patch
Javier Fernandez
Comment 2 2016-10-14 02:03:33 PDT
Comment on attachment 291593 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291593&action=review The change looks good to me. > LayoutTests/fast/css-grid-layout/grid-change-fit-content-argument.html:12 > +.verticalLR { That CSS class is already defined in the grid.css file.
Manuel Rego Casasnovas
Comment 3 2016-10-14 02:07:16 PDT
Created attachment 291598 [details] Patch
Manuel Rego Casasnovas
Comment 4 2016-10-14 02:08:02 PDT
Comment on attachment 291593 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291593&action=review Uploading new version applying suggested changes. >> LayoutTests/fast/css-grid-layout/grid-change-fit-content-argument.html:12 >> +.verticalLR { > > That CSS class is already defined in the grid.css file. Ups true, I didn't realize.
Sergio Villar Senin
Comment 5 2016-10-14 06:26:33 PDT
Comment on attachment 291598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291598&action=review > LayoutTests/fast/css-grid-layout/grid-change-fit-content-argument.html:5 > + width: fit-content; Nit: I think it'd be better to include the appropiate file and use the fit-content class so that this test could be executed in other browsers too.
Manuel Rego Casasnovas
Comment 6 2016-10-14 08:20:51 PDT
Created attachment 291634 [details] Patch for landing
Manuel Rego Casasnovas
Comment 7 2016-10-14 08:21:26 PDT
Comment on attachment 291598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291598&action=review >> LayoutTests/fast/css-grid-layout/grid-change-fit-content-argument.html:5 >> + width: fit-content; > > Nit: I think it'd be better to include the appropiate file and use the fit-content class so that this test could be executed in other browsers too. Done.
WebKit Commit Bot
Comment 8 2016-10-14 09:25:57 PDT
Comment on attachment 291634 [details] Patch for landing Clearing flags on attachment: 291634 Committed r207343: <http://trac.webkit.org/changeset/207343>
WebKit Commit Bot
Comment 9 2016-10-14 09:26:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.