1930845 - When dispatching actions the "input cancel list" is wrongly updated before the actual action is dispatched

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I noticed this problem when working on the patch on bug 1930530. It can be seen especially for pointer related actions of subtype pen which we do not have support for yet. That means that dispatching a pointerDown will fail with an unsupported error but because the input cancel list was already updated before with a new pointerUp action to reset the state of the input device, we will see another unexpected error when the release actions are automatically performed.

Per WebDriver specification we need to do:

7. Let algorithm be the value of the column dispatch action algorithm from the following table where the source type column is source type and the subtype column is equal to subtype.
source type 	subtype 	Dispatch action algorithm
"none" 	"pause" 	Dispatch a pause action
"key" 	"pause" 	Dispatch a pause action
"key" 	"keyDown" 	Dispatch a keyDown action
"key" 	"keyUp" 	Dispatch a keyUp action
"pointer" 	"pause" 	Dispatch a pause action
"pointer" 	"pointerDown" 	Dispatch a pointerDown action
"pointer" 	"pointerUp" 	Dispatch a pointerUp action
"pointer" 	"pointerMove" 	Dispatch a pointerMove action
"pointer" 	"pointerCancel" 	Dispatch a pointerCancel action
"wheel" 	"pause" 	Dispatch a pause action
"wheel" 	"scroll" 	Dispatch a scroll action

8.Try to run algorithm with arguments action object, source, global key state, tick duration, browsing context, and actions options.

9. If subtype is "keyDown", append a copy of action object with the subtype property changed to "keyUp" to input state's input cancel list.

10. If subtype is "pointerDown", append a copy of action object with the subtype property changed to "pointerUp" to input state's input cancel list. 

https://searchfox.org/mozilla-central/rev/55837bbe3e47f9b4fa91ef83a44b53823626f01d/remote/shared/webdriver/Actions.sys.mjs#1374-1377

This as well applies to other input sources as well.

This bug blocks my work on bug 1930530.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This wasn't visible yet because my patch on bug 1915798 accidentally introduced a regression in release actions due to a missing await. Due to that we did not see the error for pen as mentioned above. I'm still not sure why we even didn't see it before this patch landed.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1915798

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1930845 - [wdspec] Update release_actions/sequence_tentative.py test to run in a new tab.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1930845 - [remote] Add reverse action to "input cancel list" after trying to dispatch the action.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53ab6d394f59
[wdspec] Update release_actions/sequence_tentative.py test to run in a new tab. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/c71ce74e6512
[remote] Add reverse action to "input cancel list" after trying to dispatch the action. r=webdriver-reviewers,jdescottes

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49171 for changes under testing/web-platform/tests

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/53ab6d394f59
https://hg.mozilla.org/mozilla-central/rev/c71ce74e6512

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1930845 - [wdspec] Update release_actions/sequence_tentative.py test to run in a new tab.

Original Revision: https://phabricator.services.mozilla.com/D228817

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1930845 - [remote] Add reverse action to "input cancel list" after trying to dispatch the action.

Original Revision: https://phabricator.services.mozilla.com/D228818

Comment 11

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: For WebDriver users a race condition would be visible when Actions are performed and released.
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: None
• Risk associated with taking this patch: low
• Explanation of risk level: Just adds an await for the affected calls to methods returning a Promise.
• String changes made/needed: no
• Is Android affected?: yes

Comment 12

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: For WebDriver users a race condition would be visible when Actions are performed and released.
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: None
• Risk associated with taking this patch: low
• Explanation of risk level: Just adds an await for the affected calls to methods returning a Promise.
• String changes made/needed: No
• Is Android affected?: yes

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/f53ef84d0c2b
https://hg.mozilla.org/releases/mozilla-beta/rev/a1b3c64bc432