Skip to content
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.

Sign up for GitHub

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
Jump to bottom

[css-grid] Spaces in grid-template-areas serialization #4335

Closed
Loirooriol opened this issue Sep 19, 2019 · 5 comments
Closed

[css-grid] Spaces in grid-template-areas serialization #4335

Loirooriol opened this issue Sep 19, 2019 · 5 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-grid-1 Tested Memory aid - issue has WPT tests Tracked in DoC

Comments

@Loirooriol
Copy link
Contributor

See https://crbug.com/1005364 regarding

element.style.gridTemplateAreas = '". . a b" ". .a b"';
element.style.gridTemplateAreas; // '". . a b" ". .a b"' in Firefox
                                 // '". . a b" ". . a b"' in Blink, Webkit

According to https://drafts.csswg.org/css-grid/#valdef-grid-template-areas-string

Both the specified value and computed value of a <string> value of grid-template-areas serializes each null cell token as a single "." (U+002E FULL STOP) and each sequence of whitespace as a single space (U+0020 SPACE).

And by omission I guess that a lack of whitespace shouldn't be serialized as a space?

However, it seems confusing to me because the resolution in #3261 was against preserving the precise string when serializing grid-template-areas, and in favour of normalizing it. And according to https://drafts.csswg.org/css-grid/#valdef-grid-template-areas-string,

A sequence of whitespace, representing nothing (do not produce a token).

So if whitespace doesn't produce tokens when parsing, it doesn't seem unreasonable for me to not track whether some optional space between tokens of different type was specified or not. Then the possibilities are always adding a space separator between tokens like ". . a b" ". . a b", or only doing it if necessary (between tokens of the same type) like ". .a b" ". .a b".
@mrego
Copy link
Member

I don't have a strong opinion, but something like ". . a b" ". . a b" looks less confusing to me than ". .a b" ". .a b".

@Loirooriol
Copy link
Contributor Author

So I think the spec should say that each list of tokens is serialized into a <string> as follows:

  • Null tokens serialize as a single .
  • Named cell tokens serialize as a sequence of the code points in the name
  • The token serializations are concatenated using a single space as a separator.

@tabatkins
Copy link
Member

Yes, I think all cells should be space-separated; relying on parsing quirks (the fact that .foo and foo. will always tokenize as two separate tokens) in serialization is generally bad.

@fantasai fantasai added the Target Revision: Current Remove when done. label Nov 1, 2019
@fantasai
Copy link
Collaborator

OK, we've specified the serialization of template strings more precisely:

When serializing either the specified or computed value of a <<string>> value of 'grid-template-areas', each null cell token is serialized as a single "." (U+002E FULL STOP), and consecutive cell tokens are separated by a single space (U+0020 SPACE), with all other white space elided.

Agenda+ to confirm.

@fantasai fantasai reopened this Nov 1, 2019
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Spaces in grid-template-areas serialization, and agreed to the following:

  • RESOLVED: Specify serialization as proposed in https://github.com/w3c/csswg-drafts/issues/4335#issuecomment-548962309
The full IRC log of that discussion <dael> Topic: Spaces in grid-template-areas serialization
<dael> github: https://github.com//issues/4335
<fantasai> https://github.com//issues/4335
<TabAtkins> emilio, I'd love to with with you and jensimmons or anyone interested in this to figure out what devtools is using and how we could expose that to users.
<dael> fantasai: Want to clarify how spaces are handled. There's not compat. We said serialize between tokens we do a single space no matter if it's needed
<dael> fantasai: Want to confirm with WG
<dael> astearns: Makes sense to me
<dael> emilio: Seems weird to change string b/c we don't change string in other places
<dael> TabAtkins: You do change the string somewhat. Youd on't if parsing splits tokens correctly. But you trim spaces at end and collapse many to one
<dael> oriol: In issue #3261 we resolved against preserving percise string in favor of normalizing. Just wasn't clear on what to do with spaces
<dael> TabAtkins: Thanks
<dael> astearns: emilio?
<dael> emilio: No objections
<emilio> TabAtkins: fwiw, this is the API exposed to devtools: https://searchfox.org/mozilla-central/source/dom/webidl/Grid.webidl, via https://searchfox.org/mozilla-central/source/dom/webidl/Element.webidl#322
<dael> astearns: Other concerns?
<dael> astearns: fantasai has comment at end with proposal
<dael> astearns: Objections to this?
<dael> RESOLVED: Specify serialization as proposed in https://github.com//issues/4335#issuecomment-548962309

@mozilla-apprentice mozilla-apprentice mentioned this issue Nov 13, 2019
Closed
tabatkins added a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2020
@tabatkins
[css-grid] Remove now-disallowed variant results. Fixes w3c/csswg-dra…
439f709 
…fts#4335
@tabatkins tabatkins mentioned this issue Jul 6, 2020
Merged
@tabatkins tabatkins added Closed Accepted by CSSWG Resolution Tested Memory aid - issue has WPT tests labels Jul 6, 2020
@fantasai fantasai added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. Tracked in DoC and removed Target Revision: Current Remove when done. labels Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-grid-1 Tested Memory aid - issue has WPT tests Tracked in DoC
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mrego @tabatkins @fantasai @Loirooriol @css-meeting-bot