1709018 - Object-position percentage calculations with clamp, min and max may clamp differently if there are lengths or not, since percentage basis might be negative.

Status: VERIFIED FIXED

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

Description

[Johannes Odland]

Attachment: Screenshot_2021-05-03 CodePen - Firefox object-position percent bug.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.3 Safari/605.1.15

Steps to reproduce:

Center image using object-position calculation with clamp and percentages.

Multiple examples are given in this codepen:
https://codepen.io/johannesodland/pen/ExZqXox

Actual results:

Calculation of object position fails in some examples, and image is not centered.

Expected results:

Calculations should handle percentages correct and center image.

Comment 1

[Andrei Purice]

Reproduced on the latest versions of Firefox Nightly 90.0a1 (2021-05-19), Beta 89.0b14 and release 88.0.1.
Setting flags and adding a component for this issue in order to get the dev team involved.
If you feel it's an incorrect one please feel free to change it to a more appropriate one.

Setting a severity of S4.
Not a recent regression since it can be traced all the way back to at least Firefox 70.0.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: test-case from the reporter

Comment 3

[Emilio Cobos Álvarez (:emilio)]

On Chrome, I see the same issue if I use clamp(100%, 50%, 0%), but I get a centered image if I do clamp(100%, 50% + 0px, 0%), which feels a bit odd, at best. I think the issue here is that the percentage basis ends up being negative, depending on when you clamp() you get one result or the other.

So the reason an slightly different value from 0.5 works is that then we're not able to simplify away the lengths (if you distribute, 0.5 * (100% - 100px) + 50px is basically 50% - 50px + 50px, which is 50%. As we have clamp(<percent>, <percent>, <percent>), then we simplify it to max(100%, min(50%, 0%)), which ends up being 100%, rather than what you want, which is 50%, when there's a negative basis.

You can see this with getComputedStyle. getComputedStyle(img2).objectPosition is "100% 50%", while getComputedStyle(img3).objectPosition is "clamp(100%, 49.9999% + 0.000102997px, 0%) 50%".

The other question is why are you not just using 50% I guess, but... :)

So I think we could fix this by returning None` here, however this is a lot trickier than it seems, because for most things that are just percentages we store the resolved percentage upfront.

So I guess we could special-case object / background-position here, but whatever we do would be inconsistent with something else... I think our current behavior, while weird in this specific edge case, is somewhat reasonable.

Comment 4

[Johannes Odland]

The test case I gave was quite simplified.

We found the issue when using calc and clamp to allow the user to set a focus point on an image with object-fit: cover through custom properties --focus-x and --focus-y. This worked for most user specified values, but failed inexplicably in Firefox for some.

It doesn't seem like the specification clarifies how to handle negative basis for percentages, but I find it rather unintuitive that max(100%, min(50%, 0%)) should resolve to 100% without knowing the basis. I think max(100%, min(50%, 0%)) should resolve to the same as max(100%, min(50% + 0px, 0%)).

Maybe we should bring this issue up with the CSSWG?

Comment 5

[Johannes Odland]

I registered an issue here: https://github.com/w3c/csswg-drafts/issues/6298

I feel this is not an edge case, but will affect how and if mathematical expressions work on object-position and background-position. It is hard for authors to use mathematical expressions when the expressions sometimes are simplified to percentage only and give one result, but when not simplified give another result. Browser compatibility here would be nice :)

(I'm sorry if I come off strong. I'm trying to be precise in a language that is not my own. Thank you for all your help. I really appreciate it.)

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1709018 - Don't simplify percentages that resolve to lengths in min/max/clamp. r=#style,#layout-reviewers

Those can't be ordered at specified / computed value time, since the
percentage basis could be negative.

Needs tests of course, running through try atm.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/160919064810
Don't simplify percentages that resolve to lengths in min/max/clamp. r=boris

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29067 for changes under testing/web-platform/tests

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/160919064810

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Yeah, this is not a new regression, probably could ride the trains.

Comment 13

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 90.0a1 (2021.05.02) on Win 8.1 x64 and on macOS 11.
I can confirm this issue is fixed, I verified using Firefox 90.0b4 on Ubuntu 20.04 x64, Win 8.1 x64 and macOS 11, calculation of object position works in every examples, and images are centered.