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

Update pointer option processing #1800

Closed
wants to merge 2 commits into from
Closed

Update pointer option processing #1800

wants to merge 2 commits into from

Conversation

jrandolf-2
Copy link
Contributor

@jrandolf-2 jrandolf-2 commented Mar 4, 2024

For specific pointer types, some properties are not supported and should not be configurable. Since WebDriver already specifies properties not exposed by WebDriver must have their default set to the hardware-unsupported default recommended in https://w3c.github.io/pointerevents/#pointerevent-interface (look at each property; the interface defaults are for something else), this PR splits up validation by pointer type and removes properties for pointer types that don't support them.


Preview | Diff

For specific pointer types, some properties are not supported and should not be configurable and must be set to the default recommended in https://w3c.github.io/pointerevents/#pointerevent-interface (not the interface definition; each property has a default written in their description).
@jrandolf-2 jrandolf-2 requested a review from OrKoN March 4, 2024 13:23
Copy link
Contributor

@OrKoN OrKoN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (to only validate properties that can be set on the particular event type, and to set the default values based on other specs instead of defining WebDriver-specific defaults). Are there any WPT changes needed?

@OrKoN OrKoN requested review from whimboo and jgraham March 4, 2024 13:29
@jrandolf-2
Copy link
Contributor Author

LGTM (to only validate properties that can be set on the particular event type, and to set the default values based on other specs instead of defining WebDriver-specific defaults). Are there any WPT changes needed?

Yes, several WPT tests need to be changed.

Copy link
Member

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if @gsnedders should look at this? I think Safaridriver's implementation is nonstandard in that touch inputs will fall back to mouse inputs if there isn't touch support. If the opposite is also true (i.e. mouse becomes touch on devices with only touch input) this might cause problems.

index.html Show resolved Hide resolved
@whimboo whimboo requested a review from gsnedders March 7, 2024 12:10
@css-meeting-bot
Copy link
Member

The Browser Testing and Tools Working Group just discussed Better pointer events, and agreed to the following:

  • ACTION: Raise issue against HTML for Support for unloaded tabs [1]
The full IRC log of that discussion <AutomatedTester> topic: Better pointer events
<AutomatedTester> github: https://github.com//pull/1800
<jgraham> RRSAgent: make minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2024/03/13-webdriver-minutes.html jgraham
<AutomatedTester> jrandolf: I brought up the issue as I was working on the tests in this section
<AutomatedTester> ... there are even some items that are not used by any browser
<jgraham> q+
<AutomatedTester> ... I think we're waiting on webkit for review here
<AutomatedTester> automatedtester: shs96c could you help find someone to review
<AutomatedTester> shs96c: will do
<AutomatedTester> ack next
<AutomatedTester> jgraham (IRC): I think it is ok. It is technically a backwards incompatible change in our tests
<AutomatedTester> ... I think it's minor and hopefully a relatively safe change to make
<jrandolf> q+
<AutomatedTester> ... but it should be reviewed properly
<AutomatedTester> automatedtester: if I remember correctly this was originally done by microsoft so will reach out make make sure they are aware and will review
<jrandolf> q-
<jgraham> RRSAgent: make minutes
<jgraham> zakim, bye
<Zakim> leaving. As of this point the attendees have been JimEvans, AutomatedTester, sasha, whimboo, jrandolf, jgraham, lightning00blade, MaksimSadym, shs96c, jdescottes
<RRSAgent> I have made the request to generate https://www.w3.org/2024/03/13-webdriver-minutes.html jgraham
<jgraham> RRSAgent, bye
<RRSAgent> I see 1 open action item saved in https://www.w3.org/2024/03/13-webdriver-actions.rdf :
<RRSAgent> ACTION: Raise issue against HTML for Support for unloaded tabs [1]
<RRSAgent> recorded in https://www.w3.org/2024/03/13-webdriver-irc#T17-19-10

@gsnedders
Copy link
Contributor

I wonder if @gsnedders should look at this? I think Safaridriver's implementation is nonstandard in that touch inputs will fall back to mouse inputs if there isn't touch support. If the opposite is also true (i.e. mouse becomes touch on devices with only touch input) this might cause problems.

This is true, both ways around:

https://github.com/WebKit/WebKit/blob/fbe4a30494efb2f9a31d2a6c2866780889869737/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp#L2182-L2191

I don't think we can inject mouse events in any way on iOS, because I don't believe iOS has any support for a mouse. (Assistive Touch, as the name implies, still just dispatches touch events based on a physical mouse.) With the level we're injecting events at, we basically can't dispatch anything the OS cannot. And it turns out most existing WebDriver usage assumes there is a mouse—thus aliasing is practically a necessity for existing usage to work on a touch-only device.

AIUI, we fallback from touch to mouse because it turns out web developers ended up expecting this to work, but I don't know if it originally working was deliberate.

@jrandolf-2
Copy link
Contributor Author

@gsnedders Then would this PR cause of a problem? It seems like a non-issue for webkit.

@jrandolf-2 jrandolf-2 closed this by deleting the head repository Apr 4, 2024
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

Successfully merging this pull request may close these issues.

5 participants