web-platform-tests / interop

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

Remove a couple of scroll snap tests tied to an open spec issue from Interop 2023 #602

Closed hiikezoe closed 8 months ago

hiikezoe commented 9 months ago

As skobes told in the carryover evaluation issue for Interop 2024, some of scroll snap related tests in Interop 2023 have failed due to an open spec issue. So it would make sense to remove them from Interop 2023.

The affected tests in the list of the skobes comment are;

css/css-scroll-snap/input/snap-area-overflow-boundary.html css/css-scroll-snap/overflowing-snap-areas.html css/css-scroll-snap/scroll-snap-stop-002.html css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-align.html

And I believe css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html

is also one of such tests since the test has started failing both on Firefox and Safari since Chrome changed the test itself as well as they changed their behavior to address the open spec issue.

CC @skobes-chromium and @nt1m

nt1m commented 9 months ago

I'm in favor of removing these tests from Interop 2023.

nt1m commented 9 months ago

cc @flackr for Chromium input.

nt1m commented 9 months ago

@foolip Since the tests are failing because of an open spec issue, I would suggest removing them from Interop.

flackr commented 9 months ago

We have agreed on the linked issue that avoiding inner snap areas is expected and that the text does suggest this without prescribing exactly how it works. We haven't yet agreed on the exact behavior which is what we're attempting to do in https://github.com/w3c/csswg-drafts/issues/9187. Would it make sense / can we have a high level test that inner areas are respected / avoided in a way that doesn't prescribe the exact behavior? @skobes-chromium

Also, which tests are actually depending on this issue? As far as I can tell the ones listed do not contain or prescribe behaviors for nested snap areas - only the ones with -nested- in the name do.

flackr commented 9 months ago

The plan is to make sure that we're not including any tests related to avoiding nested snap areas in 2023, but we would like to resolve and test these in 2024. @skobes-chromium is going to make sure that we've not accidentally included these in suites in 2023 and we can make the nested tests .tentative for now.

skobes-chromium commented 9 months ago

Nested snap cases have been marked as tentative (crbug.com/1505893).

nt1m commented 9 months ago

@skobes-chromium Thanks for doing this!

@hiikezoe Can you verify that all the relevant tests have been excluded from Interop 2023?

hiikezoe commented 9 months ago

It looks like the nested cases have been excluded, but some of tests which are not nested cases still remain. It's super unclear to me what the relation ship between them is.

For example, changing-scroll-snap-align.html was tweaked in https://chromium-review.googlesource.com/c/chromium/src/+/4842691 which is, I believe, for one of the changes for nested cases.

skobes-chromium commented 9 months ago

The new cases in changing-scroll-snap-align.html are

At a glance these both seemed reasonable to expect within the existing spec so I did not move them into .tentative.

hiikezoe commented 9 months ago

If the test change was unrelated to the nest fix, that's quite unfortunate that it was done along with the fix. :/

Moreover as I commented, to me the test change isn't compliant with the existing spec text, even if I have misunderstand the spec, Chrome also had misunderstood before the nested fix, Safari has misunderstood even now. That's clearly a thing needs a spec issue to make the text clearer?

foolip commented 8 months ago

@hiikezoe are there still tests in https://wpt.fyi/results/css?label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2022-scrolling that should be excluded?

hiikezoe commented 8 months ago

No. scroll-snap-writing-mode-000.html remains there, but I don't have any concrete evidences/argues that it should also be excluded. I just saw a comment which implies it's somewhat related to the nested change, so anyways that's fine for now.

Thank you guys, @skobes-chromium, @flackr and @foolip. I am going to close this issue.