New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-grid] grid-line custom identifier auto? #2856

Closed
ewilligers opened this Issue Jul 2, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@ewilligers
Contributor

ewilligers commented Jul 2, 2018

Blink and WebKit reject grid-row-end 1 auto.
Edge and Firefox accept grid-row-end 1 auto and serialize it as 1 auto.

Blink and WebKit reject grid-row-end values span 1 auto, span auto 1 and 1 auto span.
Edge and Firefox accept these and serialize all of them as span 1 auto.

Testing with uppercase makes it clear that span is being accepted as a keyword and auto is being accepted as a custom identifier:
http://jsfiddle.net/ericwilligers/meya2q7d/

All browsers reject the following:
auto 1
span span 1, span 1 span and 1 span span
1 span auto, auto 1 span and auto span 1

The relevant grammar for <grid-line> is

<grid-line> =
  auto |
  <custom-ident> |
  [ <integer> && <custom-ident>? ] |
  [ span && [ <integer> || <custom-ident> ] ]

Are auto or span permitted as a <custom-ident>?

The custom-ident spec may be relevant:

Note: When designing grammars with <custom-ident>, the <custom-ident> should always be "positionally unambiguous", so that it’s impossible to conflict with any keyword values in the property.

I don't see a reading of the spec where span 1 auto would be permissible but not span 1 span.

@mrego mrego added the css-grid-1 label Jul 2, 2018

@mrego

This comment has been minimized.

Member

mrego commented Jul 2, 2018

Some excerpts from Grid Layout spec:

Also from CSS Values and Units spec:

There is also a related issue #494 and some old threads in www-style:

ewilligers pushed a commit to ewilligers/web-platform-tests that referenced this issue Jul 4, 2018

[css-grid] Parse various grid properties
Spec: https://drafts.csswg.org/css-grid/

These tests highlight the following 5 issues:

Spec
[cssom] [css-grid] Serialization of custom identifiers
w3c/csswg-drafts#2858

Spec
[css-grid] grid-line custom identifier auto?
w3c/csswg-drafts#2856

Edge
grid-area should reject non-integer numbers
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/18112694/

Firefox
grid-auto-columns/rows should accept multiple track-size values
https://bugzilla.mozilla.org/show_bug.cgi?id=1339672

WebKit
[CSS Grid Layout] Fix grid-{row, column, area} shorthand CSSOM serialization
https://bugs.webkit.org/show_bug.cgi?id=149890

Note that some properties such as grid and grid-template are not yet tested.
@fantasai

This comment has been minimized.

Contributor

fantasai commented Jul 24, 2018

Seems to me the spec is clear that span isn't valid as a <custom-ident>. But it does raise the question of whether auto should be similarly excluded.

@fantasai fantasai added the Agenda+ label Jul 24, 2018

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Aug 8, 2018

The Working Group just discussed grid-line custom identifier auto?, and agreed to the following:

  • RESOLVED: drop auto from grid line syntax
The full IRC log of that discussion <dael> Topic: grid-line custom identifier auto?
<dael> github: https://github.com//issues/2856
<dael> Rossen_: eric brought this and fantasai you put this on the agenda
<dael> fantasai: There was an issue against grid asking if the ID auto should be excluded from the places where we use custom ident to rep. named lined. Auto has a special meaning it doesn't resolve to a named line. We exclude span keyword, but not auto.
<dael> fantasai: Question if we should exclude auto in places we exclude span
<dael> Rossen_: I wouldn't put span and auto in same cat. for this behavior. span is forward looking directive and auto is a applied to a single item
<dael> fantasai: You cannot have...if you named a line auto you're just going to have trouble, esp. in level 2 where you can combine auto with a custom ident. You can see the grammar in the issue. If a custom ident can include auto you can write auto1 and have that resolve. I'm not sure allowing that is helpful
<dael> fantasai: Where we have span custom ident we're planning to add auto as a similar pattern. That's a grid 2 issue
<fantasai> https://github.com//issues/796#issuecomment-385547068
<dael> Rossen_: I see your point
<dael> Rossen_: I can live with it
<dael> Rossen_: Anyone from mozilla that's close to grid impl. dbaron do you have an opinion?
<dael> dbaron: No opinion. Have to ask Mats
<dael> Rossen_: It's only our impl that allow auto
<dael> fantasai: I think chances of people naming a line auto is pretty much 0 unless you're a QA person
<dael> Rossen_: How did we end up removing span but not auto?
<dael> fantasai: Don't know. I think there's some syntactic construct where one was ambig. It said span and a custom ident you could do and you want to know that you are consuming a custom ident or the span up front. If you write span span is it a keyword or an ident. So we prob did for parsing
<dael> Rossen_: We allow span int and span custom ident
<dael> fantasai: Yes and auto custom ident will be in L2
<dael> Rossen_: Okay, I'm okay with proposed change
<dael> Rossen_: Any other opinions? Or do we try to resolve?
<fantasai> s/auto custom ident/auto <custom-ident>/
<dael> Rossen_: Objections to dropping auto from grid line syntax?
<dael> RESOLVED: drop auto from grid line syntax

@css-meeting-bot css-meeting-bot removed the Agenda+ label Aug 8, 2018

ewilligers pushed a commit to ewilligers/csswg-drafts that referenced this issue Aug 9, 2018

[css-grid] Exclude 'auto' from line name
The custom-ident for line name cannot be 'auto'.

Resolved in
w3c#2856 (comment)

fixes w3c#2856

ewilligers pushed a commit to ewilligers/web-platform-tests that referenced this issue Aug 22, 2018

[css-grid] Parse various grid properties
Spec: https://drafts.csswg.org/css-grid/

These tests highlight the following 5 issues:

Spec
[cssom] [css-grid] Serialization of custom identifiers
w3c/csswg-drafts#2858

Spec
[css-grid] grid-line custom identifier auto?
w3c/csswg-drafts#2856

Edge
grid-area should reject non-integer numbers
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/18112694/

Firefox
grid-auto-columns/rows should accept multiple track-size values
https://bugzilla.mozilla.org/show_bug.cgi?id=1339672

WebKit
[CSS Grid Layout] Fix grid-{row, column, area} shorthand CSSOM serialization
https://bugs.webkit.org/show_bug.cgi?id=149890

Note that some properties such as grid and grid-template are not yet tested.

fantasai added a commit that referenced this issue Aug 24, 2018

[css-grid] Exclude 'auto' from line name
The custom-ident for line name cannot be 'auto'.

Resolved in
#2856 (comment)

fixes #2856

@fantasai fantasai removed the Needs Edits label Aug 24, 2018

ewilligers pushed a commit to ewilligers/web-platform-tests that referenced this issue Sep 4, 2018

[css-grid] Parse various grid properties
Spec: https://drafts.csswg.org/css-grid/

These tests highlight the following 5 issues:

Spec
[cssom] [css-grid] Serialization of custom identifiers
w3c/csswg-drafts#2858

Spec
[css-grid] grid-line custom identifier auto?
w3c/csswg-drafts#2856

Edge
grid-area should reject non-integer numbers
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/18112694/

Firefox
grid-auto-columns/rows should accept multiple track-size values
https://bugzilla.mozilla.org/show_bug.cgi?id=1339672

WebKit
[CSS Grid Layout] Fix grid-{row, column, area} shorthand CSSOM serialization
https://bugs.webkit.org/show_bug.cgi?id=149890

Note that some properties such as grid and grid-template are not yet tested.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 29, 2018

xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this issue Oct 30, 2018

emilio added a commit to emilio/servo that referenced this issue Nov 5, 2018

emilio added a commit to emilio/servo that referenced this issue Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment