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.

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] How to serialize the strings in grid-template-areas? #3261

Closed
tabatkins opened this issue Oct 28, 2018 · 19 comments
Closed

[css-grid] How to serialize the strings in grid-template-areas? #3261

tabatkins opened this issue Oct 28, 2018 · 19 comments

Comments

@tabatkins
Copy link
Member

'grid-template-areas' takes several strings as its value, each representing one row of the grid. These strings are then parsed into area labels.

We're basically using strings here as a convenient way to delimit the rows. They're not strings in the traditional sense, where their value contains uninterpreted text; instead, they're just a funny way of writing an ident sequence. Several implementations thus just store the property's value as a sequence of area labels, and regenerate the string on demand when needed for serialization purposes.

This means that, in Chrome/WebKit/Edge, a declaration like grid-template-areas: " a \9 b "; is serialized in computed values as just "a b", the simplest string that has the same effect. Only Firefox preserves the precise string, serializing as " a \9 b ". Testcase

Firefox is technically correct here, in that strings are preserved and serialized exactly as entered in every other case. But I don't think the other browsers are necessarily wrong here; these aren't traditional strings, and it's reasonable to "forget" what the original string was in favor of just preserving the meaning. Currently the tests for this feature are written with the assumption of simplification; here's a Moz bug about the failure and a Chrome bug they opened pointing out this violates CSSOM's serialization rules.

Should we declare one way or the other correct? Or allow them both?

@fantasai
Copy link
Collaborator

We should have interop here. I'm OK for the computed value to be simplified, it kinda makes sense to do that. Likely the specified value should not be, though.

@tabatkins
Copy link
Member Author

Right, specified values generally keep the exact value entered (maybe modulo some whitespace folding, but that shouldn't apply inside a string like this). I'm mostly just interested in computed value here.

(That said, it looks like Chrome simplifies the specified value too: updated testcase. I think we should consider that a bug on Chrome.)

@mrego
Copy link
Member

mrego commented Oct 29, 2018

(That said, it looks like Chrome simplifies the specified value too: updated testcase. I think we should consider that a bug on Chrome.)

Yes we have issues with specified values already in grid-template-columns|rows: https://bugs.chromium.org/p/chromium/issues/detail?id=716114

So it's not unexpected similar issues are present in other grid layout properties like grid-template-areas.

@ewilligers
Copy link
Contributor

Current browsers give the same string value for the specified value that they give for the computed value.

Test page: https://jsfiddle.net/ericwilligers/pw9bkh62/

Supplied Firefox specified/computed Blink/Edge/Safari specified/computed
'" a \t b "' '" a \9 b "' '"a b"'
'"c\td"' '"c\9 d"' '"c d"'
'"first ..."' '"first ..."' '"first ."'

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed How to serialize the strings in grid-template-areas?, and agreed to the following:

  • RESOLVED: serialize grid-template-areas as simplified computed value for both specified and computed value
The full IRC log of that discussion <dael> Topic: How to serialize the strings in grid-template-areas?
<dael> github: https://github.com//issues/3261
<dael> Rossen_: TabAtkins is IRC only. fantasai are you on?
<dael> ericwilligers: I can give background
<TabAtkins> Firefox preserves strings exactly. (as normal for strings) Other browsers serialize in simplified form.
<dael> ericwilligers: I wrote a wpt so serialization was similar to blink/edge/safari. FF was acting correctly, though, and the wpt was wrong.
<dael> ericwilligers: Question is what does group believe. I think spec is unclear
<dael> ericwilligers: We can defer if no one thought
<dael> heycam: If spec doesn't say else it's natural for strings to retain exact values. normalization inside string value feels a bit weird to me
<dael> ericwilligers: Other argument is 3 of 4 browsers normalize
<dael> florian: We have a micro language in the strings. It's not monolythic. normalization isn't crazy.
<dael> heycam: Did we consider if we want normalization only for computed values?
<dael> Rossen_: For computed values we're going to do simplified and for specified return actual string?
<dael> heycam: Feels more consistent with CSS
<dael> ericwilligers: I had same suggestion
<dael> Rossen_: This would require from current impl FF would need to change. I guess it's up to you if you're okay to change
<dael> heycam: Not a big deal to change. More about what makes sense
<dael> ericwilligers: If only computed value normalizes all browsers have to change. Everyone except FF changes specified
<dael> heycam: I'm arguing for what makes more sense rather from impl
<dael> Rossen_: I don't think anyone is aguing that we want to return simplied for specified.
<dael> ericwilligers: That's what 3 browsers do
<dael> Rossen_: For both speciffied and computed?
<dael> ericwilligers: Yes
<dael> Rossen_: So min implementation cost would be FF to return specified
<dael> ericwilligers: That was my original suggestion and original WPT
<dael> Rossen_: dbaron or heycam? What do you think?
<dael> dbaron: I think heycam had different opinion. Fromm change impl I think it's doable either way. For what behavior ought to be I think I'm more okay then heycam since it seems we're defining a different data type. And I'm okay saying this data type normalizes
<dael> heycam: I'm nto feeling strongly. Maybe even CSSOM guidelines about shortest values we can squint and make this fall under that guideline
<dael> Rossen_: Obj to serialize grid-template-areas as simplified computed value for both specified and computed value?
<dael> RESOLVED: serialize grid-template-areas as simplified computed value for both specified and computed value

@mrego
Copy link
Member

mrego commented Nov 8, 2018

Thinking about this a little bit more, maybe not keeping the specified value is an issue for CSS editors.
I remember @therealglazou talked about that problem in the Chromium issue I linked before.

If an user sets:

head  head   nav ..... corner
.... ....... nav aside ......
.... content nav aside ......

And then this is lost and you get back:

head head nav . corner
. . nav aside .
. content nav aside .

It might be not a big deal, but it's probably not nice I guess. 😕

@heycam
Copy link
Contributor

heycam commented Nov 8, 2018

I didn't think of that and I think it's a good point.

@emilio
Copy link
Collaborator

emilio commented Nov 8, 2018

But that already happens in a bunch of other places... I think that ship is lost already for the general case, but if somebody cares strongly enough...

@heycam
Copy link
Contributor

heycam commented Nov 8, 2018

But are there any other properties that encourage visual formatting within the string like grid-template-areas?

ewilligers pushed a commit to ewilligers/web-platform-tests that referenced this issue Nov 8, 2018
strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)
@emilio
Copy link
Collaborator

emilio commented Nov 8, 2018

Maybe not... I don't know :)

@tabatkins
Copy link
Member Author

But are there any other properties that encourage visual formatting within the string like grid-template-areas?

No, this is the only one that does something like this. Every other place we use a string, they're either passing actual textual data, or naming something.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 14, 2018
…tonly

Automatic update from web-platform-testsgrid-template-areas serialization

strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)

--
Merge pull request #13977 from ewilligers/grid-template-areas-serialization-2

grid-template-areas serialization
--

wpt-commits: da90a07f543e3307b53230a1f0a2e8b6dc72b42b, a27659a86371bb2d98e622f4e57b85a546898ade
wpt-pr: 13977
staktrace pushed a commit to staktrace/gecko that referenced this issue Nov 15, 2018
…tonly

Automatic update from web-platform-testsgrid-template-areas serialization

strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)

--
Merge pull request #13977 from ewilligers/grid-template-areas-serialization-2

grid-template-areas serialization
--

wpt-commits: da90a07f543e3307b53230a1f0a2e8b6dc72b42b, a27659a86371bb2d98e622f4e57b85a546898ade
wpt-pr: 13977
@fantasai
Copy link
Collaborator

@emilio @heycam Should the discussion here be reopened or should I edit in the resolution from #3261 (comment) ?

@emilio
Copy link
Collaborator

emilio commented Dec 11, 2018

I think given the comments here a bit more discussion may be worth it.

@mrego
Copy link
Member

mrego commented Dec 12, 2018

I'm not sure about the implementation details and why we're not doing this in Blink for any of the grid properties or how complex it would be to change this behavior. @ericwilligers do you know if the specified value is stored somewhere and we could return it?
grid-template-areas is one of the cases here, but the same happens in grid-template-rows: repeat(10, 20px) the user gets 20px 20px 20px 20px 20px 20px 20px 20px 20px 20px. Again this is an issue for CSS editors.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed How to serialize the strings in grid-template-areas?, and agreed to the following:

  • RESOLVED: Use the normalized value to serialize grid-template-areas
The full IRC log of that discussion <dael> Topic: How to serialize the strings in grid-template-areas?
<dael> github: https://github.com//issues/3261#issuecomment-446018130
<fantasai> https://github.com//issues/3261#issuecomment-436830050
<dael> fantasai: This is well illustrated by ^ comment
<dael> fantasai: grid-template-areas takes a series of strings and assigns names to different areas. When reserializing that back out do we normalize things that are eq. like compressing whitespace or do we return original string
<dael> TabAtkins: Already not returning originaly, we have compressed whitespace. It's how much compression is okay. Should we do more tracking of original author like keeping number of dots in the slot
<dael> myles: can't imagine an impl that stores original string
<dael> Rossen: Make it available to things like dev tools. It's there. Not sure if it's required and useful. I think that's the discussion
<dael> TabAtkins: 2 comments above linked eric made a table of who returns what.
<fantasai> table https://github.com//issues/3261#issuecomment-435698461
<dael> astearns: One argument to normalize is to make it easier for script to parse. Handling string as spec they have to impl browser's normalization to get useful information
<dael> emilio: Other hand this is only string that would get nromalized. All others we jsut store and return the string. This is special
<dael> Rossen: What's an example of other properties?
<dael> TabAtkins: Every other prop that takes string takes text or a name. This is one place where it's a string for non-string purposes
<dbaron> I think there might have been one other case recently where we did normalization inside a string...
<dbaron> (or maybe it was this one)
<dael> Rossen: That's why asking for example. This is the only snowflake where we're defining behavior and not something simplier
<dael> Rossen: I think current normalized format seems reasonable
<dael> emilio: Arguments for both. Either you make it easier for script to parse it or just reserve right thing useful for editors. Don't have strong preference, but we need interop
<dael> Rossen: Not sure what you mean by editors. WYSIWYG type things are using script APIs so that would be also easier
<dael> myles: Another axis is for an impl [missed]
<dbaron> I think emilio might be on a 10 second plus lag
<tantek> q?
<dael> astearns: Myles first, then emilio. If you're not talking please mute
<emilio> yeah, I think I'm lageged
<emilio> *lagged
<dbaron> I think (a) the echo is emilio and (b) the delay in the echo is showing how much lag emilio is seeing
<dael> myles: THis is speed vs memory trade off. If don't store original you can build it but that's slower
<dael> astearns: emilio go ahead
<dael> emilio: On the dev tools or the CSS editor you can set one thing and after it's applied you get something different. That's an issue reported in chromium. Whne you use repeat we're only returning computed values because we're not storing. That's a similar issue
<dbaron> s/emilio/rego/
<dael> Rossen: Your point is right on. Had this discussion at length with repeaters. I had a similar argument for keeping normalized computed for same purpose so you can represent a repeat. Consensus at that point is if you're building an editor you have enough source information to rebuild that knowledge and use the used values to infer what engine processed.
<dael> Rossen: I think consistency is something we should value between repeat and this one. In which case we'd have to stick with normalized version
<dael> astearns: emilio are you back?
<dael> emilio: Yes, I'm fine with whatever decision we take. Rossen argument is fine
<dael> astearns: Prop: Use the normalized value to serialize grid-template-areas
<dael> astearns: Obj?
<dael> RESOLVED: Use the normalized value to serialize grid-template-areas
<dael> astearns: Was this a grid issue sweep for publishing?
<dael> fantasai: NOt yet, but hoping by next week. If interested in review, these edits need to go in and there's one or two items open with small details

@fantasai
Copy link
Collaborator

I'm not sure that arguing for consistency with repeat() makes sense here. The rules for unwinding repeat() notations are for getComputedStyle, which returns used values. This question is about specified values.

fantasai added a commit that referenced this issue Dec 12, 2018
@fantasai
Copy link
Collaborator

Anyway, edited in. Reopen if there's something unclear.

@mrego
Copy link
Member

mrego commented Dec 13, 2018

I'm not sure that arguing for consistency with repeat() makes sense here. The rules for unwinding repeat() notations are for getComputedStyle, which returns used values. This question is about specified values.

Yeah I was somehow mixing both things.

The problem is that in Chromium/WebKit that for grid-template-rows: repeat(4, 20px) we return as specified value 20px 20px 20px 20px (bug #716114) and also as computed value (I'm sorry but I haven't investigated this well enough to know how complex/easy it would be to fix it).
However Firefox only does it for the computed value, and returns repeat(4, 20px) as specified value.

So one question could be if we should we change grid-template-columns|rows to be consistent to grid-template-areas regarding specified values being the same than computed values?

@fantasai fantasai added this to the css-grid-1 CR 2017-12-14+ milestone Jan 23, 2019
@fantasai
Copy link
Collaborator

@mrego I think the computed value definitely needs to maintain the repetition--for keyword repeat values like auto-fit and auto-fill it's certainly required because the resolved repeat value depends on layout. (Note that getComputedStyle does not return the computed value of grid-template-rows, it returns the used value.)

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…tonly

Automatic update from web-platform-testsgrid-template-areas serialization

strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)

--
Merge pull request #13977 from ewilligers/grid-template-areas-serialization-2

grid-template-areas serialization
--

wpt-commits: da90a07f543e3307b53230a1f0a2e8b6dc72b42b, a27659a86371bb2d98e622f4e57b85a546898ade
wpt-pr: 13977

UltraBlame original commit: 249ed8b641284bfee60972cc86588deb14c8f363
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…tonly

Automatic update from web-platform-testsgrid-template-areas serialization

strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)

--
Merge pull request #13977 from ewilligers/grid-template-areas-serialization-2

grid-template-areas serialization
--

wpt-commits: da90a07f543e3307b53230a1f0a2e8b6dc72b42b, a27659a86371bb2d98e622f4e57b85a546898ade
wpt-pr: 13977

UltraBlame original commit: 249ed8b641284bfee60972cc86588deb14c8f363
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…tonly

Automatic update from web-platform-testsgrid-template-areas serialization

strings in grid-template-areas are normalized, e.g. collapsing
whitespace to a single space, and collapsing repeated periods ('.')
to a single period.

w3c/csswg-drafts#3261 (comment)

--
Merge pull request #13977 from ewilligers/grid-template-areas-serialization-2

grid-template-areas serialization
--

wpt-commits: da90a07f543e3307b53230a1f0a2e8b6dc72b42b, a27659a86371bb2d98e622f4e57b85a546898ade
wpt-pr: 13977

UltraBlame original commit: 249ed8b641284bfee60972cc86588deb14c8f363
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants