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 /html/semantics/forms/the-input-element/selection-pointer.html from Interop 2023 #313

Closed
zcorpan opened this issue Mar 29, 2023 · 8 comments
Labels
focus area: Forms test-change-proposal Proposal to add or remove tests for an interop area

Comments

@zcorpan
Copy link
Member

zcorpan commented Mar 29, 2023

Test List

https://wpt.fyi/results/html/semantics/forms/the-input-element/selection-pointer.html?label=experimental&label=master&aligned

Rationale

Per discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1738866 this test fails in Firefox because we support multiple selections. There's a spec issue about this at w3c/selection-api#41 and it looks like there's some interest to support multiple selections also in Chromium.

cc @masayuki-nakano

@zcorpan zcorpan added the test-change-proposal Proposal to add or remove tests for an interop area label Mar 29, 2023
@karlcow
Copy link

karlcow commented Mar 30, 2023

That said the spec issue has not been discussed for 5 years.

Last meaningful updates

Should the discussion restart on the WG to test if there is new will on moving forward?

cc @rniwa too

@hsinyi
Copy link

hsinyi commented Mar 30, 2023

AFAIK, there is still recent discussion about multiple ranges in w3c/selection-api#161.

@foolip
Copy link
Member

foolip commented Mar 30, 2023

@mfreed7 can you review this proposed change?

@nt1m
Copy link
Member

nt1m commented Mar 30, 2023

This seems reasonable.

@mfreed7
Copy link

mfreed7 commented Mar 31, 2023

Hmm, I'm still not convinced about the multi-range thing being discussed on w3c/selection-api#161.

But aside from that - rather than just remove the test entirely, perhaps it's possible to instead modify the test so that it's compatible with multiple ranges? E.g. for the cases where the selection includes an unselectable item, make the test pass in either the single-range or multi-range support case?

@foolip
Copy link
Member

foolip commented Apr 27, 2023

We discussed this in #323 and the conclusion is if that if the test can be updated to tolerate either behavior for selections and ranges, everyone will be happy.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
See [1] for context. This CL makes this test pass in the case that multiple
range support is present *and* selecting across an `<input>` creates
multiple ranges. Note that in my local testing, only a single range is
created on all rendering engines (Blink, WebKit, and Gecko) when the user
selects across an `<input>`. Thus, I was not able to test the newly
added code here. I did attempt to verify that it would work correctly,
however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
See [1] for context. This CL makes this test pass in the case that
multiple range support is present *and* selecting across an `<input>`
creates multiple ranges. Note that in my local testing, only a single
range is created on all rendering engines (Blink, WebKit, and Gecko)
when the user selects across an `<input>`. Thus, I was not able to test
the newly added code here. I did attempt to verify that it would work
correctly, however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
@mfreed7
Copy link

mfreed7 commented Apr 28, 2023

We discussed this in #323 and the conclusion is if that if the test can be updated to tolerate either behavior for selections and ranges, everyone will be happy.

So I've updated the test here: web-platform-tests/wpt#39753

I wasn't able to run the actual test locally on Gecko or WebKit, but my local testing looks like it should have worked. I can tweak as needed, if someone points out an issue.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
See [1] for context. This CL makes this test pass in the case that
multiple range support is present *and* selecting across an `<input>`
creates multiple ranges. Note that in my local testing, only a single
range is created on all rendering engines (Blink, WebKit, and Gecko)
when the user selects across an `<input>`. Thus, I was not able to test
the newly added code here. I did attempt to verify that it would work
correctly, however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4492089
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137412}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 28, 2023
See [1] for context. This CL makes this test pass in the case that
multiple range support is present *and* selecting across an `<input>`
creates multiple ranges. Note that in my local testing, only a single
range is created on all rendering engines (Blink, WebKit, and Gecko)
when the user selects across an `<input>`. Thus, I was not able to test
the newly added code here. I did attempt to verify that it would work
correctly, however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4492089
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137412}
@foolip
Copy link
Member

foolip commented May 10, 2023

Results for Chrome, Firefox and Safari can now be seen at https://wpt.fyi/results/html/semantics/forms/the-input-element/selection-pointer.html?label=master&label=experimental&product=chrome&product=firefox&product=safari&aligned and all tests are passing. Closing this issue.

@foolip foolip closed this as completed May 10, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 22, 2023
…er.html to support multi-range, a=testonly

Automatic update from web-platform-tests
Update the-input-element/selection-pointer.html to support multi-range

See [1] for context. This CL makes this test pass in the case that
multiple range support is present *and* selecting across an `<input>`
creates multiple ranges. Note that in my local testing, only a single
range is created on all rendering engines (Blink, WebKit, and Gecko)
when the user selects across an `<input>`. Thus, I was not able to test
the newly added code here. I did attempt to verify that it would work
correctly, however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4492089
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137412}

--

wpt-commits: 655997d590b0d2c50a08e565e0cbab272b4b00e4
wpt-pr: 39753
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this issue May 23, 2023
…er.html to support multi-range, a=testonly

Automatic update from web-platform-tests
Update the-input-element/selection-pointer.html to support multi-range

See [1] for context. This CL makes this test pass in the case that
multiple range support is present *and* selecting across an `<input>`
creates multiple ranges. Note that in my local testing, only a single
range is created on all rendering engines (Blink, WebKit, and Gecko)
when the user selects across an `<input>`. Thus, I was not able to test
the newly added code here. I did attempt to verify that it would work
correctly, however, and I believe it will.

[1] web-platform-tests/interop#313

Change-Id: I88c88fcca47da627a0be9de518b76bbbcc07964b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4492089
Reviewed-by: Di Zhang <dizhangg@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1137412}

--

wpt-commits: 655997d590b0d2c50a08e565e0cbab272b4b00e4
wpt-pr: 39753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus area: Forms test-change-proposal Proposal to add or remove tests for an interop area
Projects
None yet
Development

No branches or pull requests

7 participants