web-platform-tests / interop

web-platform-tests Interop project
https://wpt.fyi/interop
280 stars 28 forks source link

Add css/css-nesting/supports-rule.html #683

Closed nt1m closed 3 weeks ago

nt1m commented 1 month ago

Test List

https://wpt.fyi/css/css-nesting/supports-rule.html

Rationale

This test has a different result on Chrome, Firefox and Safari. It would be nice to align the results either through implementation changes, or/and spec work. It covers nesting feature detection via @supports, which is a pretty valuable feature for web developers, it would be nice if that behaved consistently across browsers.

nt1m commented 1 month ago

cc @mdubet @andruud @Emilio

emilio commented 1 month ago

It seems ok if we fix it to match the spec :)

fwiw I think Firefox / Chrome are right. I don't think we want to make @supports syntax context-dependent (allowing relative selectors if nested). And the syntax is clear in the spec: https://drafts.csswg.org/css-conditional-4/#typedef-supports-selector-fn

andruud commented 1 month ago

It covers nesting feature detection via @supports, which is a pretty valuable feature for web developers, it would be nice if that behaved consistently across browsers.

Makes sense, but testing @supports(selector(&)) covers that feature detection capability, right? Can we drop subtests regarding relative selectors for now? (To address @emilio's concern.)

I don't think we want to make @supports syntax context-dependent (allowing relative selectors if nested).

Well, who knows, WebKit will hopefully open an issue if they feel strongly about it?

tabatkins commented 1 month ago

Yeah, this test is wrong. @supports, like @media, is not context-sensitive; it's intended to work only off of global data.

And, per the current spec, it indeed accepts only a <complex-selector>, which means relative selectors are invalid. I'm open to changing that; I don't think it was a purposeful exclusion, but I also don't care much either way, since for the purpose of feature-detection selector(&) works adequately, as @andruud noted.

So this test needs fixing either way.

Tho, hm, it's weird that Chrome is failing the first test. It's testing not selector(> .test-1): the selector(...) should fail the grammar and fall down to parsing as <general-enclosed>, which is false, and then get negated by the not to end up resolving as true. Firefox is correctly green in that case. Maybe Chrome is using the 3-valued logic from @media here (which produce a top-level unknown, which becomes false), even tho @supports specifically uses only the 2-valued logic?


Bottom line: Firefox is correct on this test currently, so we should fix the test to invert the condition on the second block. Chrome is doing something weird for the first block and will continue to fail the test after it's fixed. Safari is treating @supports as contextual and violating the specified grammar.

nt1m commented 1 month ago

@mdubet @fantasai Would you be able to fix this test?

nt1m commented 3 weeks ago

@tabatkins @andruud @Emilio The test should be fixed now, could you recheck?

nt1m commented 3 weeks ago

This is the commit that fixed it: https://github.com/web-platform-tests/wpt/commit/dd929b72a01f4e0e2b9451d17a17d23238bdce8d

emilio commented 3 weeks ago

Lgtm

andruud commented 3 weeks ago

Looks good to me as well.

Chrome is failing the first test

I've known for a while that <general-enclosed> got messed up after some refactor, https://crbug.com/40237918.