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

calc() doesn't support dividing an expression #15192

Closed
upsuper opened this issue Jan 25, 2017 · 9 comments
Closed

calc() doesn't support dividing an expression #15192

upsuper opened this issue Jan 25, 2017 · 9 comments

Comments

@upsuper
Copy link
Member

@upsuper upsuper commented Jan 25, 2017

calc(50px / 2) works, but calc(50px / (1 + 1)) doesn't work.

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Jan 25, 2017

Division is implemented as 1 / multiplication, which we probably shouldn't do.

@nox
Copy link
Member

@nox nox commented Apr 13, 2017

Do we want to keep CalcSumNode and CalcProductNode, or do we want to do a proper AST such as:

enum CalcNode {
    Value(CalcValueNode),
    BinOp(CalcValueNode, BinOp, CalcValueNode),
}

enum BinOp {
    Add, Mul, Div
}
@nox
Copy link
Member

@nox nox commented Apr 18, 2017

@upsuper https://drafts.csswg.org/css-values/#calc-syntax says / should only be followed by <number>. Do we really want to support 50px / (1 + 1)?

@nox
Copy link
Member

@nox nox commented Apr 18, 2017

Seems like the spec is wrong on this, both Firefox and WebKit don't mind the addition.

@nox
Copy link
Member

@nox nox commented Apr 18, 2017

@upsuper
Copy link
Member Author

@upsuper upsuper commented Apr 18, 2017

Yeah, dbaron is right that we should support it, especially given that Gecko already supports it, and CSSWG is going to expand that to support even more powerful expression.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented May 5, 2017

The draft has been updated with an extended syntax to support different kinds of expressions.

Taking a look at this.

@KiChjang KiChjang self-assigned this May 5, 2017
@KiChjang
Copy link
Member

@KiChjang KiChjang commented May 5, 2017

Blocked on #16728.

@wafflespeanut
Copy link
Member

@wafflespeanut wafflespeanut commented May 5, 2017

#16728 seems to fix this.

bors-servo added a commit 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 added a commit 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 added a commit 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 -->
@KiChjang KiChjang removed their assignment May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.