1387410 - make grid-row-gap and grid-column-gap no longer be reset by the grid shorthand

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In https://github.com/w3c/csswg-drafts/issues/1036 the CSSWG just decided that grid-row-gap and grid-column-gap should not be reset by the grid shorthand.

This has a bit of compatibility risk, but probably better to try sooner rather than later.  Apparently the grid *shorthand* isn't used much yet.

We should also track implementation plans in other implementations.

Comment 1

[Manuel Rego Casasnovas]

(In reply to David Baron :dbaron: ⌚️UTC-4 (away September 4-8) from comment #0)
> In https://github.com/w3c/csswg-drafts/issues/1036 the CSSWG just decided
> that grid-row-gap and grid-column-gap should not be reset by the grid
> shorthand.
> 
> This has a bit of compatibility risk, but probably better to try sooner
> rather than later.  Apparently the grid *shorthand* isn't used much yet.
> 
> We should also track implementation plans in other implementations.

This has just been fixed in Blink and WebKit,
it'd be nice if Firefox can update the behavior too.
Links:
* Blink: https://bugs.chromium.org/p/chromium/issues/detail?id=761905
* WebKit: https://bugs.webkit.org/show_bug.cgi?id=176375

Comment 2

[Manuel Rego Casasnovas]

Also we added a WPT test just in case it's useful for you too:
https://github.com/w3c/web-platform-tests/blob/master/css/css-grid-1/grid-definition/grid-shorthand-001.html

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Thanks for the heads-up Rego. I guess I can take it, should be trivial.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1387410: Stop the grid shorthand from resetting grid-gap properties.

Review commit: https://reviewboard.mozilla.org/r/176892/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176892/

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1387410: Regenerate the properties database.

Review commit: https://reviewboard.mozilla.org/r/176894/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176894/

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905090 [details]
Bug 1387410: Stop the grid shorthand from resetting grid-gap properties.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176892/diff/1-2/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905091 [details]
Bug 1387410: Regenerate the properties database.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176894/diff/1-2/

Comment 8

[Manuel Rego Casasnovas]

Also Edge seems to have updated their implementation too:
https://twitter.com/gregwhitworth/status/905460683818524672

Thanks for working on this Emilio.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905090 [details]
Bug 1387410: Stop the grid shorthand from resetting grid-gap properties.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176892/diff/2-3/

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905091 [details]
Bug 1387410: Regenerate the properties database.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176894/diff/2-3/

Comment 11

[Cameron McCormack (:heycam)]

Comment on attachment 8905090 [details]
Bug 1387410: Stop the grid shorthand from resetting grid-gap properties.

https://reviewboard.mozilla.org/r/176892/#review182602

Comment 12

[Cameron McCormack (:heycam)]

Comment on attachment 8905091 [details]
Bug 1387410: Regenerate the properties database.

https://reviewboard.mozilla.org/r/176894/#review182604

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Err, forgot that the servo bits need to go through the long path, sigh.

Comment 14

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 2 changesets with 10 changes to 10 files
remote: (e1e170a52677 modifies servo/components/style/properties/shorthand/position.mako.rs from Servo; not enforcing peer review)
remote: (1 changesets contain changes to protected servo/ directory: e1e170a52677)
remote: ************************************************************************
remote: you do not have permissions to modify files under servo/
remote: 
remote: the servo/ directory is kept in sync with the canonical upstream
remote: repository at https://github.com/servo/servo
remote: 
remote: changes to servo/ are only allowed by the syncing tool and by sheriffs
remote: performing cross-repository "merges"
remote: 
remote: to make changes to servo/, submit a Pull Request against the servo/servo
remote: GitHub project
remote: ************************************************************************
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.e_prevent_vendored hook failed
abort: push failed on remote

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Servo bits @ https://github.com/servo/servo/pull/18417

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905090 [details]
Bug 1387410: Stop the grid shorthand from resetting grid-gap properties.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176892/diff/3-4/

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8905091 [details]
Bug 1387410: Regenerate the properties database.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176894/diff/3-4/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23a9ea8f5b16
Stop the grid shorthand from resetting grid-gap properties. r=heycam
https://hg.mozilla.org/integration/autoland/rev/10bbe3749f10
Regenerate the properties database. r=heycam

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/23a9ea8f5b16
https://hg.mozilla.org/mozilla-central/rev/10bbe3749f10