1781066 - Fix handling of undefined in pointerMove and wheel actions

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[James Graham [:jgraham]]

The spec currently says it's OK to pass in undefined as x, y in pointer move and wheel scroll actions, and also deltaX and deltaY in wheel scroll actions. But it then goes on to assume that those values are integers.

The spec should change to require that those fields are not undefined, and the implementation in geckodriver/marionette updated to match.

Comment 1

[James Graham [:jgraham]]

https://github.com/w3c/webdriver/pull/1676 spec PR

Comment 2

[James Graham [:jgraham]]

Attachment: Bug 1781066 - Disallow missing pointerMove / wheel scroll coordinates,

The spec claims that undefined is allowed for these values, but later
treats them as if they're always integers. I suspect the original
intent was to default to 0, but since there seems to be interop around
rasing an exception, make geckodriver and marionette explicitly check
for valid integers and disallow missing/undefined values.

Comment 3

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

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

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

As it looks like this patch waits for bug 1746601 to be fixed? I'm going to add the dependency.

Comment 5

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

FYI the referenced pull request has been merged last week. So we are good to go with landing a patch.

Comment 6

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

James, I assume we should get this landed as well for the 0.32.0 release of geckodriver. Do you agree? If yes we should be able to get the patch reviewed?

Comment 7

[James Graham [:jgraham]]

Yes, I think we should review and land this if possible.

Comment 8

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

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 9

[Atila Butkovits]

Backed out for causing linting failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/0f26fe30b1c7db9255c9778404ad8c0679600da8

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=L0avNQM6ROGdQlzUeeSQAg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=95bc55ad64d76220aac4d559a9dd8c99757530c1

Failure log: https://treeherder.mozilla.org/logviewer?job_id=390489268&repo=autoland&lineNumber=223

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2df6d549261a
Disallow missing pointerMove / wheel scroll coordinates, r=webdriver-reviewers,whimboo

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2df6d549261a

Comment 12

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

James, the upstream wptsync failed here before and I assume this happened again? I cannot find a listed PR here. Maybe you can have a look?

Comment 13

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

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

Comment 14

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

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #13)

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

Note that this PR got already merged three days ago even there was no update from the bot.