1660332 - Make WebDriver:ElementClick Fission compatible

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task, P1)

Description

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

This bug covers the work to port all the element interaction commands to JSWindowActor. It covers the following ones:

WebDriver:ElementClear
WebDriver:ElementClick
WebDriver:ElementSendKeys

Comment 1

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

This bug and especially the command for ElementClick is actually a bit more tricky, and I hope that Nika can help us to get the right approach implemented. So what's the problem here:

1. The command does not only trigger a simulated click on the given element but also has to wait for a possible navigation to complete

2. Given that a navigation causes a new pair of actors to be created what would be the best place for navigation observing code to be located? Is it within the actor parent outside the JSWindowActor classes, or even outside the actor JSM? If it could be located inside the parent actor JSM, how should both steps (click and wait for navigation) code-wise be connected?

3. When observing the navigation we currently use navigation events (pagehide, DOMContentLoaded, pageshow, ...). Nika, you mentioned to use a web progress listener instead. What's the benefit of doing that compared to the events?

Sadly the existing Fission docs are missing to describe such a situation. So I hope that you can help us. Thanks in advance!

Comment 2

[Nika Layzell [:nika] (ni? for response)]

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

2. Given that a navigation causes a new pair of actors to be created what would be the best place for navigation observing code to be located? Is it within the actor parent outside the JSWindowActor classes, or even outside the actor JSM? If it could be located inside the parent actor JSM, how should both steps (click and wait for navigation) code-wise be connected?

3. When observing the navigation we currently use navigation events (pagehide, DOMContentLoaded, pageshow, ...). Nika, you mentioned to use a web progress listener instead. What's the benefit of doing that compared to the events?

The main advantage of using the web progress listener instead is that you can do so from the parent process, and wouldn't need to load JSWindowActor code into the content process to listen to these events from. Given you're writing testing code, you don't need to worry about per-process memory overhead as much, and can perhaps get away with using JSWindowActors which are registered to listen to these events and proxy them to the parent process.

Once the messages have arrived in the parent process, you can associate them with the BrowsingContext instance which they're coming from by getting this.browsingContext from the JSWindowActorParent. The BrowserId from that context is guaranteed to preserved across navigations, which should let you roughly approximate the event behaviour you're working with in your current patch, but in the parent process.

Sadly the existing Fission docs are missing to describe such a situation. So I hope that you can help us. Thanks in advance!

It's a bit of an odd situation, which is one of the reasons it's not covered in the docs. I'm not really aware of a mechanism for detecting whether a mouse event triggered a navigation and waiting for it even in the current system. We don't really tie together interactions and the resulting navigations like that right now. Do you currently unconditionally wait for a navigation after a click event, even if one isn't going to be triggered?

You may be interested in the testing method BrowserTestUtils.browserLoaded, which is probably the closest thing I can think of to what you're talking about. https://searchfox.org/mozilla-central/rev/d54210d490ef335b13fc1fcac817525120c8c46b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#377-405

Comment 3

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

Thanks. I will have a look at this early next week. I'm sure that there will be more questions, and maybe I have to finish bug 1612831 first which covers the move of all navigation code into the parent process.

Also it makes sense to move the other 2 element interaction commands to their own bug. As such updating the summary to only cover WebDriver:ElementClick here.

Comment 4

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

Attachment: Bug 1660332 - [wdspec] Improve tests for "Get Title" and add site-isolation test.

Depends on D88015

Comment 5

[Phabricator Automation]

Comment on attachment 9171669 [details]
Bug 1660332 - [wdspec] Improve tests for "Get Title" and add site-isolation test.

Revision D88016 was moved to bug 1660426. Setting attachment 9171669 [details] to obsolete.

Comment 6

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

Attachment: Bug 1660332 - [marionette] Port Element interaction commands to JSWindowActor.

Comment 7

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

Attachment: Bug 1660332 - [marionette] Port WebDriver:ElementClick to JSWindowActor.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d03e78f456f
[marionette] Port WebDriver:ElementClick to JSWindowActor. r=marionette-reviewers,jdescottes,maja_zf

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7d03e78f456f