The default bug view has changed. See this FAQ.

Support nested CSS calc()

RESOLVED FIXED in Firefox 48

Status

()

Core
CSS Parsing and Computation
P4
normal
RESOLVED FIXED
3 years ago
10 months ago

People

(Reporter: Gijs, Assigned: xidorn)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

unspecified
mozilla48
dev-doc-complete
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: [DocArea=CSS], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Updated

3 years ago
(Reporter)

Updated

3 years ago
Keywords: testcase

Updated

3 years ago
Priority: -- → P4
(Assignee)

Comment 1

a year ago
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().
(Assignee)

Comment 2

a year ago
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
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+
(Assignee)

Comment 4

a year ago
(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)

Comment 5

a year ago
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/
Attachment #8731079 - Flags: review?(dbaron)
(Assignee)

Updated

a year ago
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+
(Assignee)

Updated

a year ago
Attachment #8726021 - Attachment is obsolete: true
(Assignee)

Comment 7

a year ago
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/
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 8

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cd676104bc4
Keywords: checkin-needed

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9cd676104bc4
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

a year ago
Keywords: dev-doc-needed
Whiteboard: [DocArea=CSS]

Updated

a year ago
Keywords: dev-doc-needed, testcase → dev-doc-complete

Updated

10 months ago
Blocks: 913153
You need to log in before you can comment on or make changes to this bug.