[css-values] Differences between calc() handling for <length-percentage> values. #3482

emilio · 2019-01-04T13:02:28Z

I was doing some work in Gecko and realized various inconsistencies with how we treat calc(). I saw that other browsers have other (different) inconsistencies. I'm fixing some of those where it seems reasonable and we all agree. But I'm having trouble to find what the model is supposed to be.

I'll try to break this down to simpler questions...

Should `calc(%)` always be treated the same as `%`

It looks to me like it should, but in Gecko right now this is not always the case. For example, in the containing-block-size quirk, Gecko will only apply it to non-calc percentages, intentionally:

https://searchfox.org/mozilla-central/rev/ecf61f8f3904549f5d65a8a511dbd7ea4fd1a51d/layout/generic/ReflowInput.cpp#2162

<style>
  body { margin: 0 }
  div {
    height: 100%;
    float: left;
    width: 50%;
    background: green;
  }
  div + div {
    height: calc(100%);
    background: orange;
  }
</style>
<div></div>
<div></div>

Should `calc(% + 0px)` be equivalent to `calc(%)`?

I intuitively thought it should, but browsers differ here. For example, these two tables look different in blink / webkit:

<!doctype html>
<table border>                                                                                                                                                                                                     
  <tr>                                                                                                                                                                                                             
    <td style="width: calc(50%)">x</td>                                                                                                                                                                            
    <td style="width: 100px">y</td>                                                                                                                                                                                
</table>
<table border>                                                                                                                                                                                                     
  <tr>                                                                                                                                                                                                             
    <td style="width: calc(50% + 0px)">x</td>                                                                                                                                                                            
    <td style="width: 100px">y</td>                                                                                                                                                                                
</table>

I think they should render the same though (though I could be convinced otherwise).

Should `calc(<length> + 0%)` be the same as `calc(<length>)`?

I tend to think it should not, since we special-case percentages in quite a few places, and 0% is not the same as 0px everywhere (I'm pretty sure it's not the same in grid track sizing for example). Also intrinsic sizing, and this is a test passing in all browsers:

https://searchfox.org/mozilla-central/rev/ecf61f8f3904549f5d65a8a511dbd7ea4fd1a51d/testing/web-platform/tests/css/css-sizing/intrinsic-percent-non-replaced-001.html#38

I think the simplest model to get something consistent and simple implementation-wise is to keep track only whether percentages were specified, and make % + 0px always the same as %, or alternatively track both whether lengths and percentages were specified (though that's a bit more complex).

/cc @dbaron @Loirooriol @gtalbot

The text was updated successfully, but these errors were encountered:

Loirooriol · 2019-01-04T14:44:51Z

Should calc(%) always be treated the same as %

https://drafts.csswg.org/css-values-4/#calc-serialize says

If this simplification process results in only a single value (one <number>, one <dimension>, or one <percentage>), and the value being serialized is a computed value or later, serialize it just as that one value, without the calc() wrapper.

I think it would be bad if the serialization is the same but the behavior can be different.

Should calc(<length> + 0%) be the same as calc(0%)?

In general they seem clearly different, I think you meant calc(<length>) instead of calc(0%).

And I agree the 0% should not disappear, this seems already specified in https://drafts.csswg.org/css-values-4/#calc-computed-value

Where percentages are not resolved at computed-value time, they are not resolved in math functions, e.g. calc(100% - 100% + 1em) resolves to calc(1em + 0%), not to 1em. If there are special rules for computing percentages in a value (e.g. the height property), they apply whenever a math function contains percentages.

Should calc(% + 0px) be equivalent to calc(%)?

Not that sure about this one. Theoretically I think they may be considered to be different, but in practice I would probably expect e.g. CSS Tables to say they should be equivalent in your example.

Ernedar · 2019-01-04T15:17:25Z

Should calc(%) always be treated the same as %

https://drafts.csswg.org/css-values-4/#calc-serialize says

If this simplification process results in only a single value (one <number>, one <dimension>, or one <percentage>), and the value being serialized is a computed value or later, serialize it just as that one value, without the calc() wrapper.

I think it would be bad if the serialization is the same but the behavior can be different.

It depends how the calc() is threated. In my mind the calc() is threated like mathematical operation similar to (A + B) so for example if calc(A + B) is same as (A + B) so the calc(%) will be same as (%) and therefore is no matter if it is with out without those brackets. The different case is when you are using it to calculate something in real time in depency on some attribute like vw or vh. If it is the real time case, calc(%) will never be same as % because browsers are limited in rounding numbers.

emilio · 2019-01-04T16:57:11Z

Note that browsers do simplify away 0% and 0px in a bunch of cases, though I'd consider the 0% disappearing a bug.

emilio · 2019-01-04T17:07:02Z

Also:

In general they seem clearly different, I think you meant calc(<length>) instead of calc(0%).

Yes, I edited the original post :)

tabatkins · 2019-01-04T22:40:55Z

calc(%) vs %

Yeah, should be the same as much as possible, because calc(%) will serialize to %, as @Loirooriol says.

calc(% + 0px) vs calc(%)

Sure. There's no case in which these two should be different. Lengths never have magical behavior, so adding it to a % shouldn't add any complications, and in the rare cases where length+% is different than just resolving the % and adding the two together (bg-position), a 0px length is still a no-op.

More specifically, the average of calc(% + 1px) and calc(% - 1px) will, in every case I can see, be the same as as calc(%).

calc(length + 0%) vs calc(length)

No, these are different. %s can bring magic, as they sometimes resolve to intrinsic values or behave as auto. Same argument as above, but with opposite result: the average of calc(length + 1%) and calc(length - 1%) is not, in some cases, the same as calc(length) .

emilio · 2019-01-05T10:33:12Z

Alright, so you're in agreement with my intuition, that's great :)

Now we just need to fix implementations then, and maybe add a note to the spec?

This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1517511 gecko-commit: 624782f944fc58580c3f1beeb59f9ef0c6a6f127 gecko-integration-branch: autoland gecko-reviewers: heycam

…ycam This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793

This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793

…ycam This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793 --HG-- extra : moz-landing-system : lando

…ycam This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793

This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1517511 gecko-commit: 624782f944fc58580c3f1beeb59f9ef0c6a6f127 gecko-integration-branch: autoland gecko-reviewers: heycam

fantasai · 2019-01-10T21:17:11Z

The percentages of bg-position aren't special, they're relative to the size of the bg positioning area - size of the image. Which maybe looks magic, but it isn't.

fantasai · 2019-01-10T21:24:02Z

@emilio Wrt tables, see last paragraph of https://www.w3.org/TR/css-values-3/#calc-computed-value Your expectation is correct, we just let UAs do weird things in tables because tables are weird.

…comment) and reply in #3482 (comment)

fantasai · 2019-01-10T21:46:04Z

@emilio Wrt percentages resolving away, see https://www.w3.org/TR/css-values-3/#calc-computed-value particularly the example which I think makes it clear that 0% should not be resolved away.

I also rewrote the background-position example in that section to be clearer about what's going on. Lmk if you'd like anything further for the spec.

emilio · 2019-01-10T22:35:21Z

Ok, I agree with all those points. Thanks for the clarification! Will file browser bugs as needed.

tabatkins · 2019-01-11T18:05:06Z

The percentages of bg-position aren't special, they're relative to the size of the bg positioning area - size of the image. Which maybe looks magic, but it isn't.

Ah yeah, sorry, I was thinking of some other property where 0% and 0px aren't identical. (word-spacing? I don't remember)

This patch makes us handle calc with percentages when we can convert to percentages the same way we handle plain percentages in table layout. We still treat length + percentage as auto (this matches Blink / WebKit as well). There's one case we differ with Blink / WebKit, which is calc(% + 0px), which they'd treat as auto instead of a percentage. I think this is a bug on them (or at least worth some spec clarification). I filed w3c/csswg-drafts#3482 for that. In practice what that'd means for us if the WG decides that Blink / WebKit is right in that case is that we'd need to keep track of whether the calc() specifies lengths, and return false from ConvertsToPercent if so. In any case, nothing that would massively change this patch, and I think enough of an edge case that is not worth blocking on the CSSWG decision here. Though I could be convinced otherwise of course. Differential Revision: https://phabricator.services.mozilla.com/D15719 --HG-- extra : moz-landing-system : lando

This patch makes us handle calc with percentages when we can convert to percentages the same way we handle plain percentages in table layout. We still treat length + percentage as auto (this matches Blink / WebKit as well). There's one case we differ with Blink / WebKit, which is calc(% + 0px), which they'd treat as auto instead of a percentage. I think this is a bug on them (or at least worth some spec clarification). I filed w3c/csswg-drafts#3482 for that. In practice what that'd means for us if the WG decides that Blink / WebKit is right in that case is that we'd need to keep track of whether the calc() specifies lengths, and return false from ConvertsToPercent if so. In any case, nothing that would massively change this patch, and I think enough of an edge case that is not worth blocking on the CSSWG decision here. Though I could be convinced otherwise of course. Differential Revision: https://phabricator.services.mozilla.com/D15719

This is a first step to share LengthOrPercentage representation between Rust and Gecko. We need to preserve whether the value came from a calc() expression, for now at least, since we do different things depending on whether we're calc or not right now. See w3c/csswg-drafts#3482 and dependent bugs for example. That means that the gecko conversion code needs to handle calc() in a bit of an awkward way until I change it to not be needed (patches for that incoming in the next few weeks I hope). I need to add a hack to exclude other things from the PartialEq implementation because the new conversion code is less lossy than the old one, and we relied on the lousiness in AnimationValue comparison (in order to start transitions and such, in [1] for example). I expect to remove that manual PartialEq implementation as soon as I'm done with the conversion. The less lossy conversion does fix a few serialization bugs for animation values though, like not loosing 0% values in calc() when interpolating lengths and percentages, see the two modified tests: * property-types.js * test_animation_properties.html Differential Revision: https://phabricator.services.mozilla.com/D15793

…calc(%)., a=testonly Automatic update from web-platform-tests In table layout, treat calc(% + 0px) as calc(%). Per w3c/csswg-drafts#3482, in table layout, treat calc(% + 0px) as the same with calc(%). Bug: 382725, 920784 Change-Id: Ibd4face2ff6b61895e3eb6591ab1add2975e2602 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531936 Commit-Queue: David Grogan <dgroganchromium.org> Reviewed-by: David Grogan <dgroganchromium.org> Reviewed-by: Morten Stenshorne <mstenshochromium.org> Cr-Commit-Position: refs/heads/master{#643484} -- wpt-commits: 44bb330dbfab7101aa5429e7aab661a745cb839d wpt-pr: 15950 UltraBlame original commit: 4d3668f352701ae77562c36f6ba5a9cfcc1259eb

…calc(%)., a=testonly Automatic update from web-platform-tests In table layout, treat calc(% + 0px) as calc(%). Per w3c/csswg-drafts#3482, in table layout, treat calc(% + 0px) as the same with calc(%). Bug: 382725, 920784 Change-Id: Ibd4face2ff6b61895e3eb6591ab1add2975e2602 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531936 Commit-Queue: David Grogan <dgroganchromium.org> Reviewed-by: David Grogan <dgroganchromium.org> Reviewed-by: Morten Stenshorne <mstenshochromium.org> Cr-Commit-Position: refs/heads/master{#643484} -- wpt-commits: 44bb330dbfab7101aa5429e7aab661a745cb839d wpt-pr: 15950 UltraBlame original commit: ccd29dd260232732b503ebe4db7f995ab6945d25

…calc(%)., a=testonly Automatic update from web-platform-tests In table layout, treat calc(% + 0px) as calc(%). Per w3c/csswg-drafts#3482, in table layout, treat calc(% + 0px) as the same with calc(%). Bug: 382725, 920784 Change-Id: Ibd4face2ff6b61895e3eb6591ab1add2975e2602 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531936 Commit-Queue: David Grogan <dgroganchromium.org> Reviewed-by: David Grogan <dgroganchromium.org> Reviewed-by: Morten Stenshorne <mstenshochromium.org> Cr-Commit-Position: refs/heads/master{#643484} -- wpt-commits: 44bb330dbfab7101aa5429e7aab661a745cb839d wpt-pr: 15950 UltraBlame original commit: 4d3668f352701ae77562c36f6ba5a9cfcc1259eb

Per the spec issue, w3c/csswg-drafts#3482, we update the wpt to keep the percentage in `calc()` for `offset-anchor`. Differential Revision: https://phabricator.services.mozilla.com/D39552 UltraBlame original commit: d47d9f4bf9a9ea8b816534916292ca446c7543ce

Per the spec issue, w3c/csswg-drafts#3482, we update the wpt to keep the percentage in `calc()` for `offset-anchor`. Differential Revision: https://phabricator.services.mozilla.com/D39552 UltraBlame original commit: 7c6c3ac5f8fd987fd47e473e4457266498e48f1f

…calc(%)., a=testonly Automatic update from web-platform-tests In table layout, treat calc(% + 0px) as calc(%). Per w3c/csswg-drafts#3482, in table layout, treat calc(% + 0px) as the same with calc(%). Bug: 382725, 920784 Change-Id: Ibd4face2ff6b61895e3eb6591ab1add2975e2602 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531936 Commit-Queue: David Grogan <dgroganchromium.org> Reviewed-by: David Grogan <dgroganchromium.org> Reviewed-by: Morten Stenshorne <mstenshochromium.org> Cr-Commit-Position: refs/heads/master{#643484} -- wpt-commits: 44bb330dbfab7101aa5429e7aab661a745cb839d wpt-pr: 15950 UltraBlame original commit: ccd29dd260232732b503ebe4db7f995ab6945d25

…calc(%)., a=testonly Automatic update from web-platform-tests In table layout, treat calc(% + 0px) as calc(%). Per w3c/csswg-drafts#3482, in table layout, treat calc(% + 0px) as the same with calc(%). Bug: 382725, 920784 Change-Id: Ibd4face2ff6b61895e3eb6591ab1add2975e2602 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531936 Commit-Queue: David Grogan <dgroganchromium.org> Reviewed-by: David Grogan <dgroganchromium.org> Reviewed-by: Morten Stenshorne <mstenshochromium.org> Cr-Commit-Position: refs/heads/master{#643484} -- wpt-commits: 44bb330dbfab7101aa5429e7aab661a745cb839d wpt-pr: 15950 UltraBlame original commit: 4d3668f352701ae77562c36f6ba5a9cfcc1259eb

… a=testonly Automatic update from web-platform-tests Do not simplify calc(0px + 0%) into 0px Currently, when we create |Length| from calc(), and then create a CSSPrimitiveValue from that |Length|, we may drop the percentage part if it's zero. As discussed at w3c/csswg-drafts#3482, zero percentages in calcs should be preserved. Hence, this part ensures that percentage is perserved in calc(0px + 0%) Note: we may want to preserve 0% in all cases, but that leads to many test failures, so we leave the investigation to future instead. This is also preparation for crrev.com/c/1777025, which switches the implementation of InterpolableLength to a math expression to support min/max. Bug: 991672 Change-Id: I386f42a323079cce3d6ee545fa00ef289406e8bf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779721 Commit-Queue: Xiaocheng Hu <xiaochenghchromium.org> Reviewed-by: Emil A Eklund <eaechromium.org> Cr-Commit-Position: refs/heads/master{#693437} -- wpt-commits: c92eaa98a3e95a47ab21dcdb974f5d1a79e354bb wpt-pr: 18858 UltraBlame original commit: 203d6c8149371b278e332611fe8552e193373368

There should not be any behavior change between specifying a percentage using % or calc(%) per the resolution of w3c/csswg-drafts#3482. Differential Revision: https://phabricator.services.mozilla.com/D43747 UltraBlame original commit: 443a01b10c73c235a4b0b76be37fc241ca1bcc5b