w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 660 forks source link

[css-values-4] Should physical units in specified values in calc() collapse into px unit when serialized? #3741

Closed TalbotG closed 1 year ago

TalbotG commented 5 years ago

5.3.1 Compatible Units When serializing computed values, compatible units (those related by a static multiplicative factor, like the 96:1 factor between px and in, or the the computed font-size factor between em and px) are converted into a single canonical unit. Each group of compatible units defines which among them is the canonical unit that will be used for serialization.

coming from https://www.w3.org/TR/css-values-4/#compat

Now this 6 sub-tests test page: http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html (please ignore the pass or fail verdict for the 6 sub-tests)

I created this issue because no browser does what the current version of the spec explains.

In the 1st sub-test, 3 tested browsers will combine the terms 1in + 1pc into 112px. Isn't such combination going to confuse or disorient or irritate web authors?

In the 6th sub-test, 3 tested browsers will convert the term 25.4mm to 96px. Same question.

In the 2nd sub-test, all 4 tested browsers will not convert the term 25.4q to 24px. (MS-Edge 18, Safari 77 Preview and Epiphany 3.31.90 Technological Preview do not support q unit.)

In the 3rd sub-test, all 4 tested browsers will not convert the term 1.27cm to 48px.

MS-Edge 18 just like Safari 77 Preview and Epiphany 3.31.90 Technological Preview do not support q unit.

Finally, I have read https://lists.w3.org/Archives/Public/www-style/2016Mar/0331.html https://lists.w3.org/Archives/Public/www-style/2016Mar/0422.html and https://lists.w3.org/Archives/Public/www-style/2016Apr/0093.html

frivoal commented 5 years ago

As far as I can tell, the spec is very explicit and intentional about all these units getting merged and converted to pixels. I don't have a strong opinion on the matter, but it looks we decided on this explicitly.

So I guess the question is: is it time to file bugs on browsers so that implementations get aligned with the spec, or is this something implementations don't want to align on after all (in which case we need to change the spec)?

tabatkins commented 5 years ago

@frivoal Note that this is testing specified value, not computed. None of these should be combined at specified-value time, per spec; the units should just get sorted. (Except for the example with two different em values; those should be merged.)

TalbotG commented 5 years ago

this is testing specified value, not computed. None of these should be combined at specified-value time, per spec

You are probably correct. But, as edited, the spec is difficult to figure out regarding this particular matter. Even after careful and methodical re-reading, it is difficult to be sure.

the units should just get sorted.

Firefox 68 is the only browser that reliably sorts terms by their units in alphabetical order.

One question. Let's say we have calc(5mm + 6cm) What could be or should be the best/ideal serialization of specified value with such terms? a) calc(6.5cm) b) calc(65mm) // Most Europeans and Asians would go for such combination, I guess, otherwise would be enclined to go for such simplification c) calc(6cm + 5mm) // This is what the spec currently proposes: just order the terms d) calc(5mm + 6cm) // no change: the web author would not be upset about this sort of "noninterference policy". It is possibly the easiest to implement e) calc(245.669px) // what all browsers currently do right now My opinion is that each of these 5 serializations are possible, reasonable and coherent within a point of view.

AmeliaBR commented 5 years ago

None of these should be combined at specified-value time, per spec; the units should just get sorted.

Edge does it this way, except for percentages being in the wrong sorted order and no support for q units.

But they seem to be the odd ones out in not simplifying units at serialization.

Show full EdgeHTML 18.17763 results for Gerard's test Result | Test Name | Message -- | -- | -- Fail | testing calc(1pc + 1in + 1vh + 10%) | assert_equals: expected "calc(10% + 1in + 1pc + 1vh)" but got "calc(1in + 1pc + 1vh + 10%)" at Anonymous function (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:56:7) at Test.prototype.step (http://www.gtalbot.org/resources/testharness.js:1587:13) at test (http://www.gtalbot.org/resources/testharness.js:544:9) at verifySerialization (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:51:5) at startTesting (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:76:5) at Global code (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:92:3) Fail | testing calc(25.4q + 1vh + 12%) | assert_equals: expected "calc(12% + 25.4q + 1vh)" but got "calc(1in + 1pc + 1vh + 10%)" at Anonymous function (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:56:7) at Test.prototype.step (http://www.gtalbot.org/resources/testharness.js:1587:13) at test (http://www.gtalbot.org/resources/testharness.js:544:9) at verifySerialization (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:51:5) at startTesting (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:78:5) at Global code (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:92:3) Fail | testing calc(1em + 1.27cm + 13% + 3em) | assert_equals: expected "calc(13% + 1.27cm + 4em)" but got "calc(1.27cm + 4em + 13%)" at Anonymous function (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:56:7) at Test.prototype.step (http://www.gtalbot.org/resources/testharness.js:1587:13) at test (http://www.gtalbot.org/resources/testharness.js:544:9) at verifySerialization (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:51:5) at startTesting (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:80:5) at Global code (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:92:3) Pass | testing calc(5mm + 6cm) |   Pass | testing calc(7mm + 0.5in + 2pc) |   Fail | testing calc(-80px + 25.4mm) | assert_equals: expected "calc(25.4mm - 80px)" but got "calc(25.4mm + -80px)" at Anonymous function (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:56:7) at Test.prototype.step (http://www.gtalbot.org/resources/testharness.js:1587:13) at test (http://www.gtalbot.org/resources/testharness.js:544:9) at verifySerialization (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:51:5) at startTesting (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-serialization-specified-values-789.html:88:5) at Global code (http://www.gtalbot.org/BrowserBugsSection/CSS3Values/calc-seri

All 3 tested browsers will convert the term 25.4mm to 96px. Same question. … All 3 tested browsers will not convert the term 1.27cm to 48px.

Is this a typo, @TalbotG ?

Regardless of when in the process simplification happens, I would expect the same rules for all absolute length units. And that's what I'm seeing in my testing. Edge, Chrome, Firefox all treat cm and mm units using the same rules.

For Chrome and Firefox, that means that the units are left alone during serialization if and only if they are the only unit used in the calc, but are simplified to px if there is more than one absolute unit in the expression — although Chrome adds brackets in for every operation, so it also depends on the order if there are any relative units.

My opinion is that each of these 5 serializations are possible, reasonable and coherent within a point of view.

And I'm not confident that we have a web compat argument for any of them. For simple serialization, as an author I'd expect minimum modifications — whitespace normalization and maybe sorting. Even doing something like collapsing calc(100px/3) to calc(33.3333px) involves a loss of information. But if you're going to simplify that, then I don't know where the limit should be.


My test as a Codepen if anyone wants to play around with other values. It prints to the console the specified value, the serialized value read back, and the computed value (using a property that returns the regular computed value from getComputedStyle): https://codepen.io/AmeliaBR/pen/rbBzKK?editors=0012

emilio commented 5 years ago

FWIW, Gecko only normalizes absolute lengths, the rest of the units are left untouched. Relevant code is here:

https://searchfox.org/mozilla-central/rev/532e4b94b9e807d157ba8e55034aef05c1196dc9/servo/components/style/values/specified/calc.rs#70

And this particular behavior was changed here: https://bugzilla.mozilla.org/show_bug.cgi?id=1396692 (see the first comment for "old engine behavior" and "old behavior of new engine" in comparison to what's implemented now).

TalbotG commented 5 years ago

Is this a typo, @TalbotG ?

No typo. I was referring to what was happening in the (original) 4 sub-tests test page.

I have edited my original message a bit. I removed the link to MS-Edge 18 results since MS-Edge 44 is now being used (instead of MS-Edge 18) to check all the tests in wpt repository.

the units are left alone during serialization if and only if they are the only unit used in the calc, but are simplified to px if there is more than one absolute unit in the expression

Indeed, this appears to be what is happening. And that is not what the specification tried to state for serialization of terms in calc() specified values.

I still very much believe that the specification should be made more clear or clarified somehow. Maybe with more examples. And with clearer statements. The spec should explicitly state that mixed absolute units should not be resolved into px unit when serializing specified values...

TalbotG commented 5 years ago

I have modified considerably calc-serialization-002 which I will submit in a few min. I have added 6 more tests, added many more comments, etc...

ewilligers commented 5 years ago

So I guess the question is: is it time to file bugs on browsers so that implementations get aligned with the spec, or is this something implementations don't want to align on after all (in which case we need to change the spec)?

I fixed Blink's sort order for getComputedStyle result, the bug is still open for specified values.

TalbotG commented 5 years ago

AmeliaBR: (...) Chrome when serializes adds a lot of parens so from authoring might be confusing. (...) florian: If chrome inserts unnec parens it's probably wrong. (...) florian: We want chrome to fix bugs b/c having more parens in output then in input is not a thing that should happen. https://github.com/w3c/csswg-drafts/issues/3462#issuecomment-484160903

Chromium generates too often too many parentheses (...) https://bugs.chromium.org/p/chromium/issues/detail?id=947377#c12

We all agree. This is one particular behavior that Chrome software developers should fix.

tabatkins commented 5 years ago

The simplification and serialization rules are now clear about what happens here.

fantasai commented 1 year ago

calc-serialization-002.html (at least) needs updating here

TalbotG commented 1 year ago

The simplification and serialization rules are now clear about what happens here.

  • At all simplification points (immediately upon parsing, at computed-value time, at used-value time), values are absolutized as much as possible, converted to their canonical unit, and then combined together. So calc(1px + 1in) produces calc(97px) right away.

The spec § 10.13. Serialization and editor's draft § 10.13. Serialization are contradicting because still saying:

" If nodes contains any dimensions, remove them from nodes, sort them by their units, ordered ASCII case-insensitively, and append them to ret. "

Reopening. I just want the spec to be updated here...

tabatkins commented 1 year ago

The issue doesn't need to be reopened to ask for republication.

fantasai commented 1 year ago

Re-opening to expand the example that confused Gérard, we should discuss both calc(1px + 1in) and calc(1px + 1em), which have different behaviors when serializing the specified value.

fantasai commented 1 year ago

Also, we should clarify the answer @TalbotG’s question at https://github.com/web-platform-tests/wpt/pull/38245 -

The only small doubt I have is why Firefox 102.7.0 ESR and Epiphany 3.38.2 (using WebKitGTK 2.38.3) both serialize calc(4vmin + 0pt) as calc(0px + 4vmin) and not as calc(4vmin).

emilio commented 1 year ago

IIRC we used to remove zero terms of a sum and undid it. In general, removing 0px is not fully sound because it turns a length-percentage mix into just percentages, which can behave differently in tables IIRC.

In this particular case avoiding the 0px would be feasible, but it raises the question of what should remain if all the terms are zero. E.g., should calc(0pt + 0vmin + 0vmax + 0px) be 0px? Why not 0vmin? What if there's not a px term, what unit do we choose?

tabatkins commented 1 year ago

In this particular case avoiding the 0px would be feasible, but it raises the question of what should remain if all the terms are zero.

The answer is "what the simplification algorithm tells you to preserve". For a sum, it combines terms with the same unit, but doesn't touch different-unit values. If canonicalization converts them all to the canonical unit (px, here), they'll combine, otherwise they'll all stick around.

fantasai commented 1 year ago

Added a clarification note in https://github.com/w3c/csswg-drafts/commit/72d9acaac65ae60bffbf43bc6da5e8322d4bd64e ; thanks @emilio for the great explanation!