Add support for Actions in chrome scope
Categories
(Remote Protocol :: Marionette, enhancement, P5)
Tracking
(Fission Milestone:M7, firefox84 fixed)
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: automatedtester, Assigned: impossibus)
References
(Blocks 3 open bugs)
Details
(Whiteboard: [marionette-fission-mvp])
Attachments
(2 files)
It would be good to add the actions support to Chrome eventually. Original request in https://github.com/mozilla/geckodriver/issues/742
Updated•7 years ago
|
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
Assignee | ||
Comment 3•7 years ago
|
||
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•7 years ago
|
||
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.
Updated•7 years ago
|
Updated•7 years ago
|
Comment 5•4 years ago
|
||
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•4 years ago
|
||
Here the simple testcase that I was using for testing actions in chrome scope. It's based on test_mouse_action.py
.
Assignee | ||
Comment 7•4 years ago
|
||
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.
Assignee | ||
Comment 8•4 years ago
|
||
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•4 years ago
|
||
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.
Assignee | ||
Comment 10•4 years ago
|
||
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
andmultiAction
? If we can convert those usages to the spec compliant actions we would only have to keep thesingleTap
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
Assignee | ||
Comment 11•4 years ago
|
||
(In reply to Maja Frydrychowicz :maja_zf (UTC-4) (maja@mozilla.com) 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
Assignee | ||
Comment 12•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 13•4 years ago
|
||
Tracking this as part of Marionette Fission work since it blocks Bug 1534582
Comment 14•4 years ago
|
||
Tracking marionette-fission-mvp bugs for Fission Beta milestone (M7).
Assignee | ||
Updated•4 years ago
|
Comment 15•4 years ago
|
||
Pushed by mjzffr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3a4b62af9597 [marionette] Allow performActions to operate on chrome elements r=marionette-reviewers,jdescottes,whimboo
Comment 16•4 years ago
|
||
bugherder |
Comment 17•4 years ago
|
||
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•4 years ago
|
||
Emond, can you please file this as a new bug? And please provide a trace log. Thanks.
Comment 19•4 years ago
|
||
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.
Updated•2 years ago
|
Description
•