Bug 204588

Summary: [css-grid] Exclude implicit tracks from grid-template-rows/columns resolved values
Product: WebKit Reporter: Oriol Brufau <obrufau>
Component: CSSAssignee: Oriol Brufau <obrufau>
Status: REOPENED ---    
Severity: Normal CC: commit-queue, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jfernandez, kondapallykalyan, louis.acresti, macpherson, menard, pdr, rego, simon.fraser, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=1024927
Bug Depends on: 210617    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Oriol Brufau 2019-11-25 11:13:04 PST
Currently, the resolved values of grid-template-rows/columns include both implicit and explicit tracks.
However, specified values can only set explicit tracks.
Therefore, resolved values don't round-trip:

```js
gridContainer.style.gridAutoRows = "1px";
gridContainer.style.gridTemplateRows = "2px";
var cs = getComputedStyle(gridContainer);
cs.gridTemplateRows; // "2px"
gridItem.style.gridRow = "auto / 1";
gridContainer.style.gridTemplateRows = cs.gridTemplateRows; // "1px 2px"
gridContainer.style.gridTemplateRows = cs.gridTemplateRows; // "1px 1px 2px"
gridContainer.style.gridTemplateRows = cs.gridTemplateRows; // "1px 1px 1px 2px"
gridContainer.style.gridTemplateRows = cs.gridTemplateRows; // "1px 1px 1px 1px 2px"
```

https://github.com/w3c/csswg-drafts/issues/4475#issuecomment-553525609 resolved to try to exclude implicit tracks and only include explicit ones.
Comment 1 Oriol Brufau 2020-01-14 09:03:24 PST
Created attachment 387660 [details]
Patch
Comment 2 Manuel Rego Casasnovas 2020-01-15 00:27:02 PST
Comment on attachment 387660 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387660&action=review

r=me. Thanks for taking care of this.

> Source/WebCore/rendering/RenderGrid.cpp:872
> +    // Usually we have `explicitEnd <= numPositions - 1`, but the latter may be
> +    // smaller when the maximum number of tracks is reached.

Super nit: Not need to wrap this comment line, it's not that long for WebKit standards.
Comment 3 Oriol Brufau 2020-01-15 02:31:16 PST
Created attachment 387768 [details]
Patch
Comment 4 Oriol Brufau 2020-01-15 02:32:31 PST
Comment on attachment 387768 [details]
Patch

Fixed nit
Comment 5 WebKit Commit Bot 2020-01-15 03:29:30 PST
The commit-queue encountered the following flaky tests while processing attachment 387768 [details]:

inspector/worker/debugger-pause.html bug 206285 (authors: drousso@apple.com and joepeck@webkit.org)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Commit Bot 2020-01-15 03:30:03 PST
Comment on attachment 387768 [details]
Patch

Clearing flags on attachment: 387768

Committed r254561: <https://trac.webkit.org/changeset/254561>
Comment 7 WebKit Commit Bot 2020-01-15 03:30:05 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2020-01-15 03:31:18 PST
<rdar://problem/58601064>
Comment 9 Lou Acresti 2020-04-15 10:33:08 PDT
Chromium has reverted this change since it was breaking some live sites, mainly devtool-like website building apps like webflow.com and educational tools like gridcritters.com.

More info:

- https://bugs.chromium.org/p/chromium/issues/detail?id=1069866
- https://github.com/w3c/csswg-drafts/issues/4475#issuecomment-613032475

Sounds like the Chromium team are postponing the feature until there's an API for getting measurements of implicit tracks.

Disclaimer: I work at Webflow on the team responsible for the CSS Grid feature that broke due to the change in `getComputedStyle`'s API.
Comment 10 Manuel Rego Casasnovas 2020-04-16 03:13:20 PDT
(In reply to Lou Acresti from comment #9)
> Chromium has reverted this change since it was breaking some live sites,
> mainly devtool-like website building apps like webflow.com and educational
> tools like gridcritters.com.
> 
> More info:
> 
> - https://bugs.chromium.org/p/chromium/issues/detail?id=1069866
> - https://github.com/w3c/csswg-drafts/issues/4475#issuecomment-613032475
> 
> Sounds like the Chromium team are postponing the feature until there's an
> API for getting measurements of implicit tracks.
> 
> Disclaimer: I work at Webflow on the team responsible for the CSS Grid
> feature that broke due to the change in `getComputedStyle`'s API.

Yes we should revert here too, please Oriol take care of reverting the patch.

It seems "we were lucky" and this didn't make it to Safari 13.1, so just reverting in trunk would be fine I guess.
Adding Simon on CC just in case we need to do anything else.