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

Remove scroll-target-align-001.html & scroll-target-align-002.html #137

Closed
nt1m opened this issue Sep 20, 2022 · 6 comments
Closed

Remove scroll-target-align-001.html & scroll-target-align-002.html #137

nt1m opened this issue Sep 20, 2022 · 6 comments
Labels
test-change-proposal Proposal to add or remove tests for an interop area

Comments

@nt1m
Copy link
Member

nt1m commented Sep 20, 2022

Test List

  • css/css-scroll-snap/scroll-target-align-001.html
  • css/css-scroll-snap/scroll-target-align-002.html

Rationale

@nmoucht investigated, and found that this is testing an optional feature from the spec.

"If a page is navigated to a fragment that defines a target element (e.g. one that would be matched by :target, or the target of scrollIntoView()), and that element defines some snap positions, the user agent must snap to one of that element’s snap positions if its nearest scroll container is a scroll snap container. The user agent may also do this even when the scroll container has scroll-snap-type: none."

The tests are testing the last sentence which has "may" which suggests this is optional.

In fact, both tests have <meta name="flags" content="may"> in the code.

@nt1m nt1m added the test-change-proposal Proposal to add or remove tests for an interop area label Sep 20, 2022
@nt1m
Copy link
Member Author

nt1m commented Sep 21, 2022

cc @flackr @hiikezoe @theres-waldo

@hiikezoe
Copy link

@nt1m I wonder why scroll-target-align-003.html is out of the candidates, it also has <meta name="flags" content="may">.

@nt1m
Copy link
Member Author

nt1m commented Sep 21, 2022

It was passing in WebKit so we didn't look at it closely, but yes, if it is for an optional feature, we should remove it from Interop 2022. The assert does suggest that the test is for the same feature.

@foolip
Copy link
Member

foolip commented Oct 6, 2022

We discussed this in #162. There was broad agreement that we shouldn't include tests for optional behavior that some browser does not intend to implement.

I took the action item to ask for confirmation on this issue and unlabel the issue. What I'll do is go ahead an unlabel them directly, and if there's no further feedback on this issue within a week, close this issue.

@foolip
Copy link
Member

foolip commented Oct 6, 2022

Label removed from the 3 tests that have <meta name="flags" content="may"> in web-platform-tests/wpt-metadata#3074.

@nt1m
Copy link
Member Author

nt1m commented Oct 6, 2022

I think we can close this and re-open if there's any objection.

@nt1m nt1m closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

No branches or pull requests

3 participants