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.

Sign up for GitHub

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

Jump to bottom

Remove zoom-with-sign-function.html from interop-2025 #939

Open
dholbert opened this issue Mar 10, 2025 · 6 comments · May be fixed by web-platform-tests/wpt-metadata#7419
Open

Remove zoom-with-sign-function.html from interop-2025 #939

dholbert opened this issue Mar 10, 2025 · 6 comments · May be fixed by web-platform-tests/wpt-metadata#7419
Labels
test-change-proposal Proposal to add or remove tests for an interop area

Comments

@dholbert
Copy link

Test List

https://wpt.fyi/results/css/css-viewport/zoom/zoom-with-sign-function.html

Rationale

This test is currently included in interop-2025 "webcompat" focus area as part of support for the "zoom" property (proposed in #825 )

However, this test (zoom-with-sign-function) is actually testing a largely-unrelated thing, and just happens to be using the zoom property to do it. It's a test for whether sign(1em - 1px) parses and computes as-expected, which as discussed in #867 (comment) has some subtlety (and was proposed as part of a focus area over there and was rejected). Currently, Firefox rejects this syntax and fails this WPT test as a result.

In my opinion, it doesn't make sense to consider zoom interoperability as depending on how calc(sign(1em - 1px)...) behaves, so I propose we remove this test from interop-2025.

(Note that we do have a different WPT that's included in interop-2025 that also has test coverage for various zoom parsing scenarios, and in #937 I've separately proposed splitting out similar bits from that test, but leaving behind some simpler calc(...sign(...)) expressions that do work interoperably.)
@dholbert dholbert added the test-change-proposal Proposal to add or remove tests for an interop area label Mar 10, 2025
@dholbert
Copy link
Author

CC @chrishtr @nt1m does this sound OK to you? (Hopefully so, assuming you agree over on #937, since the reasoning is the same. :) )

@nt1m
Copy link
Member

nt1m commented Mar 11, 2025

Seems ok to me

@dholbert
Copy link
Author

Thanks! I'll write a PR to remove the test from the interop set.

@nt1m
Copy link
Member

nt1m commented Mar 14, 2025

Folks from Chromium haven't replied yet it seems

@dholbert
Copy link
Author

dholbert commented Mar 14, 2025 •
edited
Loading

Oh right, I misremembered that they had replied.

@chrishtr would you mind signing off on this? (Basically the same tests/reasoning as #937 as noted above; it's a test that was included due to having zoom in the name, but is really testing an edge-casey parser behavior that's unrelated to zoom in particular. The tests themselves are essentially identical to the ones I spun out in #937.)

Thanks!

@chrishtr
Copy link
Contributor

LGTM!

dholbert added a commit to dholbert/wpt-metadata that referenced this issue Mar 14, 2025
@dholbert
Remove zoom-with-sign-function.html from the webcompat focus area.
06eb44a 
This fixes web-platform-tests/interop#939
@dholbert dholbert linked a pull request Mar 14, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove zoom-with-sign-function.html from the webcompat focus area. dholbert/wpt-metadata
3 participants
@dholbert @nt1m @chrishtr