Last Comment Bug 968761 - Support nested CSS calc()
: Support nested CSS calc()
Status: RESOLVED FIXED
[DocArea=CSS]
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
P4 normal with 1 vote (vote)
: mozilla48
Assigned To: Xidorn Quan [:xidorn] (UTC+10)
:
: Jet Villegas (:jet)
Mentors:
data:text/html;charset=utf-8,<!DOCTYP...
Depends on:
Blocks: css3test
  Show dependency treegraph
 
Reported: 2014-02-06 04:42 PST by :Gijs
Modified: 2016-05-20 02:28 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
proposed patch (2.60 KB, patch)
2016-03-02 18:15 PST, Xidorn Quan [:xidorn] (UTC+10)
dbaron: feedback+
Details | Diff | Splinter Review
MozReview Request: Bug 968761 - Treat nested calc() as plain parenthesis. (58 bytes, text/x-review-board-request)
2016-03-15 23:43 PDT, Xidorn Quan [:xidorn] (UTC+10)
dbaron: review+
Details | Review

Description User image :Gijs 2014-02-06 04:42:32 PST

    
Comment 1 User image Xidorn Quan [:xidorn] (UTC+10) 2016-03-01 17:22:36 PST
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 User image Xidorn Quan [:xidorn] (UTC+10) 2016-03-02 18:15:33 PST
Created attachment 8726021 [details] [diff] [review]
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 User image David Baron :dbaron: ⌚️UTC-8 2016-03-02 21:35:19 PST
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 User image Xidorn Quan [:xidorn] (UTC+10) 2016-03-03 19:17:46 PST
(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 User image Xidorn Quan [:xidorn] (UTC+10) 2016-03-15 23:43:22 PDT
Created attachment 8731079 [details]
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 User image David Baron :dbaron: ⌚️UTC-8 2016-03-16 15:40:39 PDT
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 User image Xidorn Quan [:xidorn] (UTC+10) 2016-03-17 20:14:11 PDT
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 9 User image Carsten Book [:Tomcat] 2016-03-18 08:08:49 PDT
https://hg.mozilla.org/mozilla-central/rev/9cd676104bc4

Note You need to log in before you can comment on or make changes to this bug.