Closed Bug 968761 Opened 7 years ago Closed 5 years ago

Support nested CSS calc()

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: Gijs, Assigned: xidorn)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=CSS])

Attachments

(1 file, 1 obsolete file)

No description provided.
Keywords: testcase
Priority: -- → P4
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().
Attached patch proposed patch (obsolete) — Splinter Review
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
Attachment #8726021 - Flags: feedback?(dbaron)
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.
Attachment #8726021 - Flags: feedback?(dbaron) → feedback+
(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
Assignee: nobody → quanxunzhen
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
Attachment #8731079 - Flags: review?(dbaron) → review+
Attachment #8726021 - Attachment is obsolete: true
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/
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9cd676104bc4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Keywords: dev-doc-needed
Whiteboard: [DocArea=CSS]
You need to log in before you can comment on or make changes to this bug.