Bug 163434 - [css-grid] Changing the argument on fit-content() doesn't cause the grid to be relayout
Summary: [css-grid] Changing the argument on fit-content() doesn't cause the grid to b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 60731
  Show dependency treegraph
 
Reported: 2016-10-14 01:39 PDT by Manuel Rego Casasnovas
Modified: 2016-10-14 09:26 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.87 KB, patch)
2016-10-14 01:42 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (5.82 KB, patch)
2016-10-14 02:07 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch for landing (5.92 KB, patch)
2016-10-14 08:20 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.