Intent to Implement and Ship: Unprefix CSS Grid Layout gutter properties

100 views
Skip to first unread message

Manuel Rego Casasnovas

unread,
Jan 24, 2018, 7:06:13 AM1/24/18
to blink-dev
**Contact emails**

re...@igalia.com

**Spec**

https://www.w3.org/TR/css-grid-1/#gutters

**Summary**

Rename gutter properties to remove "grid-" prefix:
* grid-gap => gap
* grid-row-gap => row-gap
* grid-column-gap => column-gap

Note that column-gap already exists and is used by css-multicol.
The parsing needs to be updated as now the default value is "normal".
The old (prefixed) properties names will be kept working as aliases.

I'm just sending the intent because this is adding 2 new CSS properties
(gap and row-gap) and I'll need API owners approval to land the patch.

**Motivation**

This was resolved by the CSSWG past summer
(https://github.com/w3c/csswg-drafts/issues/1696) and there isn't any
good reason to not doing it.

**Interoperability risk**

* Firefox: Public support
* Edge: Shipped
* Safari: No public signals
* Web developers: No signals

It's a small change, that adds 2 new CSS properties: gap and row-gap
(that already exist in the grid prefixed version).

Edge is already shipping this change, and Firefox is willing to
implement it too.
In addition, I'll take care to implement it on WebKit too and I don't
foresee any issue to get it accepted there.

**Compatibility risk**

The old properties will be kept as aliases so this won't break any content.
Even when the new initial value is "normal", for Grid Layout that's
resolved to 0px, so it won't cause any change.

**Ongoing technical constraints**

Talking about the CSS parser, it seems we cannot have 2 aliases for the
same property and column-gap has already -webkit-column-gap as alias.
Anyway we could implement the unprefixed properties as a shorthands,
which is what the spec actually states
(https://drafts.csswg.org/css-align/#gap-legacy):
* grid-row-gap must be treated as a shorthand for the row-gap property
* grid-column-gap must be treated as a shorthand for the column-gap property
* grid-gap must be treated as a shorthand for the gap property

**Will this feature be supported on all six Blink platforms (Windows,
Mac, Linux, Chrome OS, Android, and Android WebView)?**

Yes.

**OWP launch tracking bug**

https://bugs.chromium.org/p/chromium/issues/detail?id=761904

**Link to entry on the Chrome Platform Status**

https://www.chromestatus.com/features/4986266210795520

**Requesting approval to ship?**

Yes.

Philip Jägenstedt

unread,
Jan 24, 2018, 9:19:31 AM1/24/18
to Manuel Rego Casasnovas, blink-dev
LGTM1

If https://wpt.fyi/css/css-grid doesn't have any tests for this, or perhaps tests for the old syntax, can you add/update those? If the old names are actually gone from the spec then negative tests for that would be good, which we'd fail until we remove them. (Adding such tests can reveal that everyone fails, in which case adding things to the spec might be a better idea.)

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/d0d66015-9365-071c-6453-011c2b0dcad1%40igalia.com.

Manuel Rego Casasnovas

unread,
Jan 24, 2018, 10:28:58 AM1/24/18
to Philip Jägenstedt, blink-dev

On 24/01/18 15:19, Philip Jägenstedt wrote:
> If https://wpt.fyi/css/css-grid doesn't have any tests for this, or
> perhaps tests for the old syntax, can you add/update those? If the old
> names are actually gone from the spec then negative tests for that would
> be good, which we'd fail until we remove them. (Adding such tests can
> reveal that everyone fails, in which case adding things to the spec
> might be a better idea.)

Yes there are a few tests already (that we're not passing now):
http://w3c-test.org/css/css-grid/alignment/grid-gutters-001.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-003.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-005.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-007.html

In any case, all the tests that we write for this work will be done in
WPT directly.

The old names are still valid, so we don't need to make them negative tests.

Bye,
Rego

>
> On Wed, Jan 24, 2018 at 7:06 PM Manuel Rego Casasnovas <re...@igalia.com
> <mailto:re...@igalia.com>> wrote:
>
> **Contact emails**
>
> re...@igalia.com <mailto:re...@igalia.com>

Rick Byers

unread,
Jan 24, 2018, 11:22:58 AM1/24/18
to Manuel Rego Casasnovas, Philip Jägenstedt, blink-dev
LGTM2

On Wed, Jan 24, 2018 at 10:28 AM, Manuel Rego Casasnovas <re...@igalia.com> wrote:

On 24/01/18 15:19, Philip Jägenstedt wrote:
> If https://wpt.fyi/css/css-grid doesn't have any tests for this, or
> perhaps tests for the old syntax, can you add/update those? If the old
> names are actually gone from the spec then negative tests for that would
> be good, which we'd fail until we remove them. (Adding such tests can
> reveal that everyone fails, in which case adding things to the spec
> might be a better idea.)

Yes there are a few tests already (that we're not passing now):
http://w3c-test.org/css/css-grid/alignment/grid-gutters-001.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-003.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-005.html
http://w3c-test.org/css/css-grid/alignment/grid-gutters-007.html

In any case, all the tests that we write for this work will be done in
WPT directly.

The old names are still valid, so we don't need to make them negative tests.

They're valid in our implementation but not according to the spec, right?  WPT is supposed to match specs, so I think it's reasonable to have negative test cases for an API whenever one is removed from the spec (by shipping a removed API chromium is not spec compliant).  Basically we have outstanding debt which can only be resolved by either removing the old API or specifying that the old API is still required in a spec somewhere (everything else is just wishful thinking).  Plus our WPT tooling can help us prioritize these cases: as philip says, WPTs which all browsers fail suggest a possible spec bug, while WPTs which fail in chromium but pass in at least two other engines suggest a likely opportunity for us to easily fix.

Manuel Rego Casasnovas

unread,
Jan 24, 2018, 11:28:07 AM1/24/18
to Rick Byers, Philip Jägenstedt, blink-dev


On 24/01/18 17:22, Rick Byers wrote:
> LGTM2
>
> On Wed, Jan 24, 2018 at 10:28 AM, Manuel Rego Casasnovas
> <re...@igalia.com <mailto:re...@igalia.com>> wrote:
> The old names are still valid, so we don't need to make them
> negative tests.
>
>
> They're valid in our implementation but not according to the spec
> <https://www.w3.org/TR/css-grid-1/#change-2016-grid-gap>, right?  WPT is
> supposed to match specs, so I think it's reasonable to have negative
> test cases for an API whenever one is removed from the spec (by shipping
> a removed API chromium is not spec compliant).  Basically we have
> outstanding debt which can only be resolved by either removing the old
> API or specifying that the old API is still required in a spec somewhere
> (everything else is just wishful thinking).  Plus our WPT tooling can
> help us prioritize these cases: as philip says, WPTs which all browsers
> fail suggest a possible spec bug, while WPTs which fail in chromium but
> pass in at least two other engines suggest a likely opportunity for us
> to easily fix.

Sorry but I didn't example myself properly.
They're still valid according to the spec
(https://www.w3.org/TR/css-align-3/#gap-legacy):
"For compatibility with legacy content, those legacy property names must
be supported as aliases"

They were moved to css-align as they are now generalized (without the
prefix), and they're not anymore in css-grid spec. But we need to keep
them working.

Bye,
Rego

>
>
> Bye,
>   Rego
>
> >
> > On Wed, Jan 24, 2018 at 7:06 PM Manuel Rego Casasnovas <re...@igalia.com <mailto:re...@igalia.com>
> > <mailto:re...@igalia.com <mailto:re...@igalia.com>>> wrote:
> >
> >     **Contact emails**
> >
> >     re...@igalia.com <mailto:re...@igalia.com>
> <mailto:re...@igalia.com <mailto:re...@igalia.com>>
> >
> >     **Spec**
> >
> >     https://www.w3.org/TR/css-grid-1/#gutters
> <https://www.w3.org/TR/css-grid-1/#gutters>
> >
> >     **Summary**
> >
> >     Rename gutter properties to remove "grid-" prefix:
> >     * grid-gap => gap
> >     * grid-row-gap => row-gap
> >     * grid-column-gap => column-gap
> >
> >     Note that column-gap already exists and is used by css-multicol.
> >     The parsing needs to be updated as now the default value is
> "normal".
> >     The old (prefixed) properties names will be kept working as
> aliases.
> >
> >     I'm just sending the intent because this is adding 2 new CSS
> properties
> >     (gap and row-gap) and I'll need API owners approval to land
> the patch.
> >
> >     **Motivation**
> >
> >     This was resolved by the CSSWG past summer
> >     (https://github.com/w3c/csswg-drafts/issues/1696
> <https://github.com/w3c/csswg-drafts/issues/1696>) and there isn't any
> <https://drafts.csswg.org/css-align/#gap-legacy>):
> >     * grid-row-gap must be treated as a shorthand for the row-gap
> property
> >     * grid-column-gap must be treated as a shorthand for the
> column-gap
> >     property
> >     * grid-gap must be treated as a shorthand for the gap property
> >
> >     **Will this feature be supported on all six Blink platforms
> (Windows,
> >     Mac, Linux, Chrome OS, Android, and Android WebView)?**
> >
> >     Yes.
> >
> >     **OWP launch tracking bug**
> >
> >     https://bugs.chromium.org/p/chromium/issues/detail?id=761904
> <https://bugs.chromium.org/p/chromium/issues/detail?id=761904>
> >
> >     **Link to entry on the Chrome Platform Status**
> >
> >     https://www.chromestatus.com/features/4986266210795520
> <https://www.chromestatus.com/features/4986266210795520>
> >
> >     **Requesting approval to ship?**
> >
> >     Yes.
> >
> >     --
> >     You received this message because you are subscribed to the Google
> >     Groups "blink-dev" group.
> >     To view this discussion on the web visit
> >   
>  https://groups.google.com/a/chromium.org/d/msgid/blink-dev/d0d66015-9365-071c-6453-011c2b0dcad1%40igalia.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/d0d66015-9365-071c-6453-011c2b0dcad1%40igalia.com>.
> >
>
> --
> You received this message because you are subscribed to the Google
> Groups "blink-dev" group.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/b659a3c1-6796-9673-b7e7-b98df3ec02ca%40igalia.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/b659a3c1-6796-9673-b7e7-b98df3ec02ca%40igalia.com>.
>
>

Rick Byers

unread,
Jan 24, 2018, 1:51:22 PM1/24/18
to Manuel Rego Casasnovas, Philip Jägenstedt, blink-dev
Ah, thanks for the clarification!  I only searched the grid spec, but that makes perfect sense.  Sorry for the confusion.

Chris Harrelson

unread,
Jan 25, 2018, 10:20:13 PM1/25/18
to Rick Byers, Manuel Rego Casasnovas, Philip Jägenstedt, blink-dev
LGTM3

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY959Q1tCbaNTKkyCu4ifHQWcHYwuxsUmpD9PKC%2BLT3dFA%40mail.gmail.com.

PhistucK

unread,
Jan 26, 2018, 4:27:21 AM1/26/18
to Chris Harrelson, Rick Byers, Manuel Rego Casasnovas, Philip Jägenstedt, blink-dev
Will your implemented gap apply to CSS columns as well?
Can a columns container be a grid as well, or is that illegal and unsupported (which one wins?)?


PhistucK

Manuel Rego Casasnovas

unread,
Jan 26, 2018, 8:21:19 AM1/26/18
to PhistucK, Chris Harrelson, Rick Byers, Philip Jägenstedt, blink-dev

On 26/01/18 10:26, PhistucK wrote:
> Will your implemented gap apply to CSS columns as well?

Yes, that's already happening in Edge, and I guess it's the expected
behavior if you set "gap", you're setting "column-gap" too.

> Can a columns container be a grid as well, or is that illegal and
> unsupported (which one wins?)?

Not, that's not possible.

It used to be the following sentence in Grid Layout spec:
"the column-* properties in the Multi-column Layout module [CSS3COL]
have no effect on a grid container."

That has been removed recently [1], because a new text has been added in
css-multicol properties [2] and it was redundant:
"Applies to: block containers [3] except table wrapper boxes"

Bye,
Rego

[1] https://github.com/w3c/csswg-drafts/pull/1560
[2] https://drafts.csswg.org/css-multicol/#the-number-and-width-of-columns
[3] https://drafts.csswg.org/css-display-3/#block-container
Reply all
Reply to author
Forward
0 new messages