1365886 - Add support for Actions in chrome scope

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement, P5)

Description

[David Burns :automatedtester]

It would be good to add the actions support to Chrome eventually.

Original request in https://github.com/mozilla/geckodriver/issues/742

Comment 1

[chase]

Hello,
I raised the request on GitHub.

I'd like to take a stab at implementing, and have setup the development environment, following the instructions (more or less) at https://wiki.mozilla.org/User:Mjzffr/New_Contributors.

Could I get some mentorship in terms of which  files (or zipcode there of) that I should start looking at to implement?

Thanks,
Chase

chase  at   cwpdev.com

Comment 2

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

Maja, can you mentor this bug?

Comment 3

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

You're welcome to take a look. I think the main reason we haven't implemented this yet is that we haven't yet thought about how to maintain and possibly share state (seen elements, input state, inputs to cancel) between driver.js and listener.js.

The main entry-point for actions is here: http://searchfox.org/mozilla-central/source/testing/marionette/driver.js#1774-1813 and this is where you should handle the case of chrome scope. The other actions-related functions below are the legacy actions implementation -- ignore them. 

The corresponding content-scope actions code is in listener.js: http://searchfox.org/mozilla-central/source/testing/marionette/listener.js#886-908 and http://searchfox.org/mozilla-central/source/testing/marionette/listener.js#627-631

I think ato might have some opinions regarding how to extend actions to work in chrome scope (it should be an "extension command" I guess) and how it should work.

Comment 4

[Andreas Tolfsen ❲:ato❳]

First of all, I don’t know if there is a pressing need for actions in chrome context: we mostly get by with the high-level Element Click and Element Send Keys commands to interact with XUL elements at the moment.  I can see how having actions available might be useful to write more sophisticated Firefox UI interaction tests, but it’s not clear to me if we have anyone lined up to write such tests?

For keyboard input, the current implementation in testing/marionette/action.js should work fine also for chrome space.  It ends up calling down to testing/marionette/event.js, our synthetic interaction module, which Element Send Keys also uses.  This might be the easiest thing to do first.

For mouse interaction I’m unsure if the same assumptions about the location of the cursor that we make for content context will be valid for chrome.  Specifically we need to handle the case where a chrome context mouse click ends up interacting with web content/the viewport.  Not sure how we should handle this, but maybe an error could be returned.

When the mouse is moved over the browser, we also potentially could end up causing DOM events to be triggered in content context, which isn’t desired but may be a natural consequence.

When it comes to shared state, I don’t think we would have to share the known web element cache between chrome and content, but it’s a bit of an open question whether input state, inputs to cancel, and generally any state associated with the individual pointer sources.

If we do want to share this state, I would suggest standing up a service in chrome context (testing/marionette/driver.js), much like our cookie service (testing/marionette/cookie.js), to keep all this information.  This means our current content implementation of actions needs to change to use this service.

Comment 5

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

To get actions to work in chrome scope we would have to fix bug 1669176 first.

Further there seems to be a mix-up of WebElement types:

1601969089139	Marionette	DEBUG	2 -> [0,27,"WebDriver:FindElement",{"using":"id","value":"link"}]
1601969089139	Marionette	TRACE	[36] Parent actor created
1601969089139	Marionette	TRACE	[36] Child actor created for window id 25
1601969089141	Marionette	DEBUG	2 <- [1,27,null,{"value":{"chromeelement-9fc5-4b51-a3c8-01716eedeb04":"0d036a9d-9bee-c04d-a106-06d676b78220"}}]
1601969089142	Marionette	DEBUG	2 -> [0,28,"WebDriver:GetElementProperty",{"id":"0d036a9d-9bee-c04d-a106-06d676b78220","name":"localName"}]
1601969089142	Marionette	DEBUG	2 <- [1,28,null,{"value":"a"}]
1601969089143	Marionette	DEBUG	2 -> [0,29,"WebDriver:PerformActions",{"actions":[{"parameters":{"pointerType":"mouse"},"type":"pointer","id":"pointer_id","actions":[{"y":0,"x":0,"type":"pointerMove","origin":{"element-6066-11e4-a52e-4f735466cecf":"0d036a9d-9bee-c04d-a106-06d676b78220"}},{"button":0,"type":"pointerDown"},{"button":0,"type":"pointerUp"}]}]}]
1601969089144	Marionette	DEBUG	2 <- [1,29,{"error":"invalid argument","message":"Expected 'origin' to be undefined, \"viewport\", \"pointer\", or an element, got: <a id=\"link\" href=\"#\">","stacktrace":"WebDriverError@chrome://marionette/content/error.js:181:5\nInvalidArgumentError@chrome://marionette/content/error.js:310:5\naction.PointerOrigin.get@chrome://marionette/content/action.js:371:11\nfromJSON@chrome://marionette/content/action.js:744:44\nfromJSON@chrome://marionette/content/action.js:868:34\nfromJSON@chrome://marionette/content/action.js:809:48\nperformActions@chrome://marionette/content/actors/MarionetteFrameChild.jsm:392:20\nreceiveMessage@chrome://marionette/content/actors/MarionetteFrameChild.jsm:142:31\n"},null]

Somehow the ChromeWebElement reference is turned into a WebElement reference for WebDriver:PerformActions. Note that other commands don't explicitly use the ChromeWebElement reference but just its value. As it looks like this should be a problem with the Marionette Python client.

Maja, as I can remember right you wanted to check the actions vs legacy actions bit. Maybe it's something you are interested in to investigate? We should figure out if it makes sense for us to add this support now, or simply convert legacy actions to JSWindowActors for now. From my point of view the latter might make more sense even that we know we actually don't want to continue support for these.

Comment 6

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

Attachment: Simple Marionette testcase

Here the simple testcase that I was using for testing actions in chrome scope. It's based on test_mouse_action.py.

Comment 7

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

I've started looking at this and I've figured out with the Python client. Not done yet, but I think it might not be too hard to get actor-based actions working in chrome scope.

That being said, the tests that use legacy actions use tap/touch, and we don't have that implemented in the spec-compliant actions yet, so it might make sense to port legacy actions over to JSWindowActors regardless if it's not much work.

More on this soon.

Comment 8

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Yay, I have spec-compliant actions working in chrome scope with JSWindowActors (in a basic test). So next, I'll check if I can get AWSY tests working -- that is the only thing that depends on chrome-scope actions.

While I'm here, here's a breakdown of legacy actions usage:

• actionChain, used in:
  • testing/awsy/awsy/awsy_test_case.py depends on this in chrome scope
  • layout/base/tests/marionette/test_accessiblecaret_cursor_mode.py (content only)
  • layout/base/tests/marionette/test_accessiblecaret_selection_mode.py (content only)
  • Marionette unit tests like testing/marionette/harness/marionette_harness/tests/unit/test_key_actions.py
• multiAction
  • Nothing is using this except Marionette Python client (gestures)
• singleTap - this is content scope only, so not relevant to this bug. Tests that depend on tap are:
  • layout/base/tests/marionette/test_accessiblecaret_cursor_mode.py
  • layout/base/tests/marionette/test_accessiblecaret_selection_mode.py
  • testing/marionette/harness/marionette_harness/tests/unit/test_accessibility.py

singleTap is pretty entwined with the rest of legacy actions, so I don't think we can drop legacy actions until we add touch support to spec-compliant actions.

Comment 9

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

Thanks for this analysis Maja! And it's great to hear that you got actions working in chrome scope with the actors!

Something which would be good to know is what should happen with actionChain and multiAction? If we can convert those usages to the spec compliant actions we would only have to keep the singleTap from legacy actions. That would be at least a good step forward.

So as it sounds lets convert singleTap to use the actor implementation, and get the others removed / replaced with webdriver actions - in the hope this all will work smoothly.

Comment 10

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

I commented on Bug 1534582 regarding AWSY.

My conclusion here is that we'll get chrome support for actions "for free" once Bug 1669169 is resolved.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #9)

Thanks for this analysis Maja! And it's great to hear that you got actions working in chrome scope with the actors!

Something which would be good to know is what should happen with actionChain and multiAction? If we can convert those usages to the spec compliant actions we would only have to keep the singleTap from legacy actions. That would be at least a good step forward.

Yeah, we should be able to convert actionChain usages to performActions. multiAction doesn't really matter because nothing is using it as far as I know, but performActions should be able to cover that, too.

So as it sounds lets convert singleTap to use the actor implementation, and get the others removed / replaced with webdriver actions - in the hope this all will work smoothly.

That's worth a shot. Filed Bug 1670295

Comment 11

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

(In reply to Maja Frydrychowicz :maja_zf (UTC-4) ([e-mail]) from comment #10)

My conclusion here is that we'll get chrome support for actions "for free" once Bug 1669169 is resolved.

That and we'll need some minor modifications to actions.js and the Marionette Python client.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e13224bc905ec5d6640ba039246bc978c313c7f4

Comment 12

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Attachment: Bug 1365886 - [marionette] Allow performActions to operate on chrome elements

Comment 13

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Tracking this as part of Marionette Fission work since it blocks Bug 1534582

Comment 14

[Chris Peterson [:cpeterson]]

Tracking marionette-fission-mvp bugs for Fission Beta milestone (M7).

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a4b62af9597
[marionette] Allow performActions to operate on chrome elements r=marionette-reviewers,jdescottes,whimboo

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/3a4b62af9597

Comment 17

[Emond Papegaaij]

This change broke our selenium tests. We used this code (Java) to open the context menu, click on an item and close the menu again:

new Actions(driver).contextClick(element).build().perform();
driver.setContext("chrome");
driver.findElement(By.id("keyhub_topicus_nl-menuitem-_context-fill")).click();
driver.findElement(By.id("keyhub_topicus_nl-menuitem-_context-fill")).sendKeys(Keys.ESCAPE);
driver.setContext("content");

This worked on FF 83, but on 84 it gives:

Element <menuitem id="keyhub_topicus_nl-menuitem-_context-fill" class="menuitem-iconic"> is not reachable by keyboard

We had to change the sendKeys to the following to fix this:

new Actions(driver).sendKeys(Keys.ESCAPE).build().perform();

Comment 18

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

Emond, can you please file this as a new bug? And please provide a trace log. Thanks.

Comment 19

[Emond Papegaaij]

I've filed bug 1685291. Can you explain what kind of trace log you are looking for and how to get that? The logging of our Firefox docker container selenium/standalone-firefox-debug gives no information on this. The stacktrace of our test is also not very interesting as it's simply a call to WebElement.sendKeys going into the command execution of the RemoteWebDriver and returning with an error.

Comment 20

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

I would suggest that we follow-up on the other bug.