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-values-4][cssom] <resolution> serializes to 'dppx', not 'x' #2241

Closed
csnardi opened this issue Jan 30, 2018 · 7 comments
Closed

[css-values-4][cssom] <resolution> serializes to 'dppx', not 'x' #2241

csnardi opened this issue Jan 30, 2018 · 7 comments
Labels

Comments

@csnardi
Copy link
Contributor

csnardi commented Jan 30, 2018

After #461, Values 4 states that:

All <resolution> units are compatible, and 'x' is their canonical unit.

However, the CSSOM spec for serializing <resolution> says:

The resolution in dots per CSS pixel serialized as per <number> followed by the literal string "dppx".

The Typed OM spec also makes no mention of 'x', only 'dppx'.

Should the canonical unit for <resolution> be changed back to 'dppx'? Changing all of these serializations to 'x' from 'dppx' might not be web-compatible, and would result in differences in serialization among browsers that have/have not added 'x' as a unit of resolution.

@csnardi
Copy link
Contributor Author

csnardi commented Jan 30, 2018

This also raises: how should 'x' be serialized alone? Should it become 'dppx'? Or remain as 'x'?

@fantasai fantasai added cssom-1 Current Work css-values-4 Current Work labels Jan 30, 2018
@upsuper
Copy link
Member

upsuper commented Jan 31, 2018

Looks like x and dppx are actually the same unit. Probably we should just change Vaues 4 to state dppx is the canonical unit rather than x, given it is mentioned in other places, and it feels more meaningful.

@csnardi
Copy link
Contributor Author

csnardi commented Jan 31, 2018

Makes sense; but then would e.g. '1x' be serialized to '1dppx'? It seems the previous resolution didn't have a clear answer to this; the relevant discussion from #461 is below:

<dael> fantasai: We've never had alias units, so what do you get if you serialize it back out.
<dael> leaverou: dpx?
<leaverou> s/dpx/dppx/
<dael> TabAtkins: It's only image resolution and webkit image set, so let's see what webkit does and do what they say. It'll b e x or dppx.
<dael> smfr: Pretty sure it's x
<dael> fantasai: If you put in dppx do you get out x?
<dael> TabAtkins: In specified style no. We don't need fancy. They're different units with a 1 to 1 ratio. In computed when you canonicalize you go to probably x.

This seems to suggest that there are two different units rather than one unit with two names, which doesn't seem to be clear in the spec. I'm also not sure if this makes the most sense, as they really are the same unit, just specified in two different ways.

This is relevant for Chrome unprefixing image-set, as our implementation should match the spec before unprefixing (bug is https://bugs.chromium.org/p/chromium/issues/detail?id=807653).

@css-meeting-bot
Copy link
Member

The Working Group just discussed dppx vs x in serialization, and agreed to the following:

  • RESOLVED: dppx is canonical unit
The full IRC log of that discussion <fantasai> Topic: dppx vs x in serialization
<fantasai> TabAtkins: When Webkit introduced -webkit-? function
<fantasai> TabAtkins: They used an 'x' unit instead of using a <resolution>
<fantasai> TabAtkins: <resolution> at the time didn't include 'x'
<fantasai> TabAtkins: Wanted to make 'x' a resolution, too
<fantasai> TabAtkins: equivalent to dppx
<smfr> s/-webkit-? function/-webkit-image-set/
<fantasai> TabAtkins: For serialization, it seemed that ? function would be more often used than resolution queries in OM
<fantasai> TabAtkins: So I wanted to make 'x' the canonical unit
<fantasai> TabAtkins: Our implementers expressed some concern about this, thought we should use 'dppx'
<fantasai> Florian: If you have people with compat concerns, then let's minimize the concern
<fantasai> astearns: Any objections to using dppx and canonical unit
<TabAtkins> github: https://github.com//issues/2241
<fantasai> RESOLVED: dppx is canonical unit

@foolip
Copy link
Member

foolip commented May 10, 2018

@tabatkins should this have the "needs testcase" label or what will traxk the eventual creation/updating of tests?

@csnardi I guess you mean to add tests as part of the implementation?

@csnardi
Copy link
Contributor Author

csnardi commented May 10, 2018

Yep, the plan is to add tests while implementing. It looks like @emilio also might be adding tests in https://bugzilla.mozilla.org/show_bug.cgi?id=1460655, but I think that's for the Firefox version of https://github.com/w3c/web-platform-tests/blob/master/css/mediaqueries/test_media_queries.html and not the WPT version.

@emilio
Copy link
Collaborator

emilio commented May 11, 2018

Oh I didn't know that that test was in WPT. I'll modify the WPT test too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants