968761 - Support nested CSS calc()

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

This can be useful when combined with CSS Variables, since it would allow authors to write calc() in custom properties and reference them in other calc().

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: proposed patch

As mentioned in www-style [1], nested calc() is identical to just parentheses. This seems to be pretty straightforward to implement.

The main issue is that there is already a reftest in CSSWG's test repo for this [2], so I don't want to add another one, but I don't think we are able to import individual reftests there. Not sure what can I do.

[1] https://lists.w3.org/Archives/Public/www-style/2016Mar/0017.html
[2] https://hg.csswg.org/test/file/a72171340915/css-values-3/calc-in-calc.html

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8726021 [details] [diff] [review]
proposed patch

(In reply to Xidorn Quan [:xidorn] (UTC+8) from comment #2)
> As mentioned in www-style [1], nested calc() is identical to just
> parentheses. This seems to be pretty straightforward to implement.

An important question is whether the way you've implemented is conformant.  In particular, if somebody accesses the specified style, e.g.:
  element.style.width = "calc(3 * calc(2px + 3px))"
  console.log(element.style.width));
are they supposed to get the result with or without the nested "calc"?

The spec should be clear on this, one way or the other.  But I don't think the current spec is useful because I don't think the Serialization section is firmly based in reality, as I described in:
https://lists.w3.org/Archives/Public/www-style/2016Feb/0184.html

> The main issue is that there is already a reftest in CSSWG's test repo for
> this [2], so I don't want to add another one, but I don't think we are able
> to import individual reftests there. Not sure what can I do.

It should be easy enough to modify layout/reftests/w3c-css/import-tests.py to do what you want.  But if you import extra tests, that's fine too.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

(In reply to David Baron [:dbaron] ⌚️UTC+8 from comment #3)
> Comment on attachment 8726021 [details] [diff] [review]
> proposed patch
> 
> An important question is whether the way you've implemented is conformant. 
> In particular, if somebody accesses the specified style, e.g.:
>   element.style.width = "calc(3 * calc(2px + 3px))"
>   console.log(element.style.width));
> are they supposed to get the result with or without the nested "calc"?

Given this reply, I think it would likely be conformant :)

https://lists.w3.org/Archives/Public/www-style/2016Mar/0048.html

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

Attachment: MozReview Request: Bug 968761 - Treat nested calc() as plain parenthesis.

Review commit: https://reviewboard.mozilla.org/r/40351/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40351/

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8731079 [details]
MozReview Request: Bug 968761 - Treat nested calc() as plain parenthesis.

https://reviewboard.mozilla.org/r/40351/#review37133

::: layout/style/nsCSSParser.cpp:7576
(Diff revision 1)
> +  return aToken.mType == eCSSToken_Function &&
> +    (aToken.mIdent.LowerCaseEqualsLiteral("calc") ||
> +     aToken.mIdent.LowerCaseEqualsLiteral("-moz-calc"));

please line up the "(" on the second line with the "a" on the first line

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8731079 [details]
MozReview Request: Bug 968761 - Treat nested calc() as plain parenthesis.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40351/diff/1-2/

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cd676104bc4

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9cd676104bc4