1773264 - Refactor actions support

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task)

Description

[James Graham [:jgraham]]

This will make it easier to add new input types in the future, and bring us closer to the current design of the spec.

Comment 1

[James Graham [:jgraham]]

Attachment: Bug 1773264 - Refactor actions support,

Move from having lots of switch/case statements to dispatch the
correct actions to a more normal OO setup where each action type is
represented by a class with a static fromJSON method for construction,
and a dispatch() method for emitting the event.

The state is also passed around explictly rather than being stored in
a module global. This will allow us to have e.g. different state per
session.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96fbfb8cd08a
Refactor actions support, r=webdriver-reviewers,whimboo

Comment 3

[James Graham [:jgraham]]

Attachment: Bug 1773264 - Ensure test uses same mouse input source for all actions,

By default we're creating a new pointer for each Action, and by the third one it's no longer
being treated as a mouse-type pointer, so we get the wrong events.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4521e5cab98e
Refactor actions support, r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/86256ec81bca
Ensure test uses same mouse input source for all actions, r=webdriver-reviewers,whimboo

Comment 5

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

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

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/4521e5cab98e
https://hg.mozilla.org/mozilla-central/rev/86256ec81bca

Comment 7

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

Upstream PR merged by moz-wptsync-bot

Comment 8

[Andrew Nicols]

FYI this change introduced some new error messages and was not noted in the 103 release notes (https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/103).

Specifically the amount of error checking around Origin co-ordinates and the fromJSON function has significantly increased and whilst these error conditions are largely correct, some of them were working previously and are arguably not defined as errors in the specification.

In our case we were using a click action to select an Option element in a Select element, which lead to an "Origin element is not displayed" error. This behaviour was previously working, and still works in other browsers (Chrome).

There is a comment stating that this behaviour is currently not defined in the spec (https://github.com/w3c/webdriver/issues/1642 was opened to discuss it), so I'm surprised that the decision was made to turn it into an Error in Firefox.

In our case it's an easy fix, but I wonder if there are other possible cases where the origin is legitimately not visible.

Comment 9

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

Thanks Andrew for bringing that up. I'll leave the answer to James who worked on the refactoring.

Comment 10

[James Graham [:jgraham]]

I think from a spec lawyering point of view "these are arguably not defined as errors in the specification" is missing the point: the specification just fails to consider that there might be an element without bounding client rects, so everything in that case is undefined behaviour.

In general the idea that you can have a coordinate origin that doesn't correspond to a visible point on the screen doesn't seem reasonable.

Presumably in the case of <option> you want something like "the position it would be displayed, if it were being displayed". But even that doesn't quite work: if e.g. the select has a scrollable UI then there are multiple possible postions the element could be in.

So I guess the question is: what's the expected behaviour here? Is it expected that the user does some work to ensure that the element is actually in view, or is the idea that we have some way to dispatch events to specific option elements without actually dealing with the browser-specific select UI? What exactly is Chrome doing in these cases?

I agree this is a regression and we should prioritize a fix, but I don't think we should do so without a clear understanding of what we're trying to achieve.

If you happen to have a testcase we can use to investigate this behaviour, that would be helpful.

Comment 11

[Julian Descottes [:jdescottes]]

Hi Andrew,

As mentioned by James, can you provide a test case? You mentioned that the amount of "error checking" changed compared to older FF versions or to Chrome, but I'm not sure then what was the behavior and what regressed. Was it already erroring before, but in a different way?

We should create a new bug to track this so feel free to either answer here or directly file a new bug. Thanks.

Comment 12

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

Andrew can you please have a look at the replies from James and Julian? Thanks!

Comment 13

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

Note that I've filed bug 1805146 for the mentioned regression.