Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-values] calc()'s divisions should support numeric expressions #1241

Closed
nox opened this issue Apr 18, 2017 · 14 comments
Closed

[css-values] calc()'s divisions should support numeric expressions #1241

nox opened this issue Apr 18, 2017 · 14 comments

Comments

@nox
Copy link
Contributor

nox commented Apr 18, 2017

https://drafts.csswg.org/css-values/#calc-syntax

The spec says calc() supports only numbers after /, thus excluding things like (1 + 1). Both Firefox and WebKit support the latter, so the spec should be changed.

Cc @dbaron @SimonSapin

@dbaron
Copy link
Member

dbaron commented Apr 18, 2017

I believe the spec used to allow for dividing by numerical-only expressions; I'm not sure why that changed. (We also want to expand to allowing more in the future, so I don't see the harm in allowing the implemented thing now.)

@FremyCompany
Copy link
Contributor

Works in Edge too. If it works everywhere, I agree we should update the spec.

FremyCompany added a commit that referenced this issue Apr 19, 2017
Another option would be to allow any calc-value again, and let the prose that follows disambiguate like we do for the * products.
@FremyCompany
Copy link
Contributor

The grammar also doesn't allow the calc( calc(...) ) we resolved to accept, so it should be updated to include that option as well. Depending on how we decide to fix this issue, it might depend on previous PR so I'll refrain from making the change now.

@tabatkins
Copy link
Member

The OP's bug is invalid; the grammar in https://drafts.csswg.org/css-values/#calc-syntax definitely allows numeric expressions in the denominator, and the type-checking in https://drafts.csswg.org/css-values/#calc-type-checking is referring to "resolved types". Expressions like (1 + 1) have a "resolved type" of <number>.

François's issue is also invalid - a calc() will always resolve to either a <number>, <percentage>, or <dimension>, so nested calc()s are definitely allowed per the grammar.

@FremyCompany
Copy link
Contributor

@tabatkins Oh, yes, you are right about my second issue!

The first issue was valid before the spec changed. The spec seems to have been accidentally changed by my pull request, I don't know why... I made the commit in a branch. Did someone merge it?

@FremyCompany
Copy link
Contributor

Here is what I see in the spec now:

The syntax of a calc() function is:
<calc()> = calc( <calc-sum> )
<calc-sum> = <calc-product> [ [ '+' | '-' ] <calc-product> ]*
<calc-product> = <calc-value> [ '*' <calc-value> | '/' <calc-number-value> ]*
<calc-value> = <number> | <dimension> | <percentage> | ( <calc-sum> )
<calc-number-sum> = <calc-number-product> [ [ '+' | '-' ] <calc-number-product> ]*
<calc-number-product> = <calc-number-value> [ '*' <calc-number-value> | '/' <calc-number-value> *
<calc-number-value> = <number> | ( <calc-number-sum> )

which is what my pull request was adding. I am fairly confused I must say.

@FremyCompany
Copy link
Contributor

I think the bikeshed watcher screwed up. The spec was just updated to include my PR but the last commit in master is still 6 days old. Next time the watcher will regenerate the spec, my PR will disappear.

https://github.com/w3c/csswg-drafts/blob/master/css-values/Overview.bs

@tabatkins
Copy link
Member

Sigh, yes, this confused me too. According to @plinss this is due to a bug in Mercurial's bridging. Okay, nm, let's just merge the PR.

(@plinss suggests not doing PRs from the main repo, as it causes this. Do it in your fork like normal.)

@FremyCompany
Copy link
Contributor

Thanks for investigating! Good to know, too...

The downside is that it means I can't use the edit button on github to fix the specs. That seems mildly annoying. On my work PC it's a no-brainer but at home I find it easier.

@plinss
Copy link
Member

plinss commented Apr 19, 2017

@FremyCompany you can hit the "Fork" button on GitHub to fork the repo into your own account, then you can use the "Edit" button in that repo

@fantasai fantasai changed the title [css-values] calc()'s divisions should support more things [css-values] calc()'s divisions should support numeric expressions Apr 19, 2017
triple-underscore added a commit to triple-underscore/triple-underscore.github.io that referenced this issue Apr 19, 2017
emilio added a commit to emilio/servo that referenced this issue May 4, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 4, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
emilio added a commit to emilio/servo that referenced this issue May 4, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 4, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
emilio added a commit to emilio/servo that referenced this issue May 4, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 4, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
emilio added a commit to emilio/servo that referenced this issue May 4, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 4, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
emilio added a commit to emilio/servo that referenced this issue May 5, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 5, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this issue May 5, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
emilio added a commit to emilio/servo that referenced this issue May 5, 2017
This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward.

[1]: w3c/csswg-drafts#1241
bors-servo pushed a commit to servo/servo that referenced this issue May 5, 2017
style: Rewrite calc to be cleaner and support arbitrary expressions.

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16728)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 6, 2017
…rbitrary expressions (from emilio:recalc); r=waffles

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

Source-Repo: https://github.com/servo/servo
Source-Revision: 7fc01437f4c8935951add61a76230131134382f8

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : b89b3a24eaa1f1c213f22e74068912c941896e02
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this issue May 8, 2017
…rbitrary expressions (from emilio:recalc); r=waffles

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

Source-Repo: https://github.com/servo/servo
Source-Revision: 7fc01437f4c8935951add61a76230131134382f8
fantasai added a commit that referenced this issue May 17, 2017
@fantasai
Copy link
Collaborator

Agenda+ to confirm with the WG that this goes in L3. Since L4 is going to have unit algebra in the denominator, it seems to me that this edit was intended for L3).

@astearns astearns reopened this May 23, 2017
@tabatkins
Copy link
Member

Yes, this is just a mistake in the L3 spec (my fault, I think). It matches current impls.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed CSS Values and Units: numeric expression denominators, and agreed to the following resolutions:

  • RESOLVED: Numeric expressions are explicitly allowed in denominators of calc() expressions
The full IRC log of that discussion <fantasai> Topic: CSS Values and Units: numeric expression denominators
<fantasai> github topic: https://github.com//issues/1241
<myles> fantasai: we originally intended the denominator of calc() to have a numeric expression
<myles> fantasai: like calc(13 / (5 + 3))
<myles> fantasai: we don't like units because unit math is hard
<myles> fantasai: numeric math is ok though
<myles> fantasai: that isn't in the spec but we do have implementations. so we should add this to level 3
<myles> fantasai: level 4 doesn't make sense because level 4 will include all the unit math and everything which would be a superset of this
<myles> fantasai: but we need a resolution if we want to add it to level 3
<myles> Rossen: yes please
<myles> Rossen: <expresses support>
<myles> Rossen: any objections?
<myles> RESOLVED: Numeric expressions are explicitly allowed in denominators of calc() expressions

@FremyCompany
Copy link
Contributor

I think edits were already made, closing the issue.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…rbitrary expressions (from emilio:recalc); r=waffles

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

Source-Repo: https://github.com/servo/servo
Source-Revision: 7fc01437f4c8935951add61a76230131134382f8

UltraBlame original commit: d5aa4f2a3c9bd947292ce93a82c37c552cfe6b1e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…rbitrary expressions (from emilio:recalc); r=waffles

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

Source-Repo: https://github.com/servo/servo
Source-Revision: 7fc01437f4c8935951add61a76230131134382f8

UltraBlame original commit: d5aa4f2a3c9bd947292ce93a82c37c552cfe6b1e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…rbitrary expressions (from emilio:recalc); r=waffles

This improves Servo's calc support compliant with[1], and makes it cleaner and
more straight-forward. (also fixes #15192)

[1]: w3c/csswg-drafts#1241

Source-Repo: https://github.com/servo/servo
Source-Revision: 7fc01437f4c8935951add61a76230131134382f8

UltraBlame original commit: d5aa4f2a3c9bd947292ce93a82c37c552cfe6b1e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants