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

Changes to .tentative tests #48258

Closed
annevk opened this issue Sep 19, 2024 · 5 comments
Closed

Changes to .tentative tests #48258

annevk opened this issue Sep 19, 2024 · 5 comments

Comments

@annevk
Copy link
Member

annevk commented Sep 19, 2024

In 15c5ae8 @dizhang168 with review from @tkent-google made quite major changes to a tentative test contributed by WebKit.

The rationale given was that the behavior wasn't standardized. But that's what .tentative means.

Could you please revert this change?

cc @foolip

@dizhang168
Copy link
Member

The original test was only for Selection.direction, which is a new not fully standardized test that Webkit added. I agree it should be kept as tentative.
55dd5fe

However, the part I removed was for an addendum that Gecko made to test multi click (single, double and triple clicks).
3af40b4
04e0f3e

I looked into this and the click behavior is not specified. Because of OS differences, it is also hard to have this standardized:
w3c/selection-api#70 (comment)
w3c/selection-api#177 (comment)

When this patch was landed, I did cc the OP to make sure they are aware and so they can correct me:
#47043 (comment)

If we still feel strong about keeping this part of the test in WPT, I can add it as a separate new file called selection-direction-on-clicks.tentative.html and make it clear that shipping selection.direction doesn't mean this test should become non-tentative.

@annevk
Copy link
Member Author

annevk commented Sep 19, 2024

If the expectation is that it can become non-tentative modulo a set of subtests, putting those subtests in a separate file seems like the best way forward indeed. Unless you have actual confirmation from the writer of the tests that removing them is okay. (Probably best done as a PR against WPT itself with them as reviewer.)

@dizhang168
Copy link
Member

Thanks for the feedback. I have created the PR for it and asked @sefeng211's review: #48267

Note: I agree with you it is not good to remove a test without the original writer's approval. I did cc and gotten their approval in the original PR: #47043 (comment)

@dizhang168
Copy link
Member

Closing this as I have re-added the subtests as "selection/Selection-direction-on-clicks.tentative.html".

@annevk
Copy link
Member Author

annevk commented Sep 20, 2024

Thanks @dizhang168 and apologies for not entirely appreciating the sequence of events here. Glad it was a misunderstanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants