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

Description Manuel Rego Casasnovas 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
Comment 1 Manuel Rego Casasnovas 2016-10-14 01:42:32 PDT 
Created attachment 291593 [details]
Patch
Comment 2 Javier Fernandez 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.
Comment 3 Manuel Rego Casasnovas 2016-10-14 02:07:16 PDT 
Created attachment 291598 [details]
Patch
Comment 4 Manuel Rego Casasnovas 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.
Comment 5 Sergio Villar Senin 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.
Comment 6 Manuel Rego Casasnovas 2016-10-14 08:20:51 PDT 
Created attachment 291634 [details]
Patch for landing
Comment 7 Manuel Rego Casasnovas 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.
Comment 8 WebKit Commit Bot 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>
Comment 9 WebKit Commit Bot 2016-10-14 09:26:01 PDT 
All reviewed patches have been landed.  Closing bug.