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] Specify how to handle negative percentage basis in mathematical expressions #6298

Closed
johannesodland opened this issue May 19, 2021 · 8 comments

Comments

@johannesodland
Copy link

Some properties like object-position can have a negative percentage basis.

This causes an issue where some browsers calculates/simplifies clamp(100%, 50%, 0%) to 100% ignoring the negative percentage basis, but solves clamp(100%, 50% 0px, 0%) to 50%. With a percentage basis of -100px the original equation is equal to clamp(-100px, -50px, 0px) which would solve to -50px. Solving the equation without accounting for the negative basis creates unexpected behaviour.

(See https://bugzilla.mozilla.org/show_bug.cgi?id=1709018).

It would be helpful if the spec specified how to handle negative percentage basis in mathematical expressions https://drafts.csswg.org/css-values-4/#calc-func

@Loirooriol
Copy link
Contributor

See #4227 (comment):

you resolve as much as you can at each step but % can't be resolved at computed value time unless they are a terminal type for the property. If lengths have to wait.

So it's a bug if the implementation assumes that 0% <= 50% <= 100% and doesn't wait until the percentages can be resolved into a length. This is already implied in the spec, https://drafts.csswg.org/css-values/#calc-simplification

If root is an operator node that’s not one of the calc-operator nodes, and all of its children are numeric values with enough information to computed the operation root represents, return the result of running root’s operation using its children, expressed in the result’s canonical unit.

@emilio
Copy link
Collaborator

emilio commented May 19, 2021

I think reading the spec to the letter, we should never simplify percentages. However that's a bit of a hard one to fix with properties that only parse percentages because that requires preserving the calculation a lot longer than browsers are now.

@emilio
Copy link
Collaborator

emilio commented May 19, 2021

Hah, I mid-aired with oriol :)

@Loirooriol
Copy link
Contributor

But properties that don't resolve percentages into another (non-number) type, like opacity, compare percentages as percentages. So you can resolve immediately.

@tabatkins
Copy link
Member

No, @emilio is right - the spec (mistakenly) never simplifies %s in min()/max(). It assumes they'll turn into another type, at which point they can simplify.

I should fix that to specify that %s which would resolve to another type in the context shouldn't simplify away in this case, but %s which stand on their own can.

@tabatkins
Copy link
Member

That said, the OP is indeed invalid - the spec definitely does specify that you don't resolve a min() or max() in that case until simplification can change the %s into lengths, at which point you get the correct values.

tabatkins added a commit that referenced this issue May 20, 2021
…at %s will always block simplification, and add a Note making the % behavior more explicit. #6298
@tabatkins
Copy link
Member

I did just now add a small note making the behavior of percentages clearer, tho.

@johannesodland
Copy link
Author

@tabatkins

This was indeed specified in the spec, and I just missed it. The spec makes sense as it is, but the added note is really helpful.
Thank you for clearing this up :)

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

4 participants