Closed Bug 1354578 Opened 7 years ago Closed 2 years ago

[meta] Replace legacy actions with WebDriver action commands in Marionette client


(Remote Protocol :: Marionette, enhancement, P3)

Version 3


(Not tracked)



(Reporter: impossibus, Unassigned)


(Blocks 1 open bug)


(Keywords: meta)


(2 obsolete files)

The new spec-compliant actions endpoints, performActions and releaseActions (Bug 1292178), currently support action chains with key, mouse and pause. It's best for clients to be using these new endpoints. Moreover, we believe only that only internal Marionette tests are using the old actions endpoints (actionChain, multiAction, singleTap), so let's remove them.
This changeset shows what else needs to be removed to get a green try push with no references to legacyaction.js:

Try push:

Conclusion: a few tests under dom/layout rely on actions, as well as a few Marionette unit tests for accessibility and switching window content. We should be able to write modify these tests to use the new actions implementation.
Depends on: 1387461
Priority: -- → P3
I will start working on this bug. Right now I'm reading the Marionette documentation, since I'm fairly new around here and not entirely familiar with it. If anyone has tips on how to get started, I will surely appreciate it!
Hi Vinicius. As discussed via email it's great to see that you have interest to start on this bug. To get familiar with Marionette make sure to read everything on the following page:

Then as first step you will have to identify which of the existent Marionette tests in tree make use of the legacy actions commands. That way we will see how much work it will be, and if we better have to split-up this bug into smaller portions of work.
(In reply to Andreas Tolfsen 「:ato」 from comment #4)
> You might want to also see
> Contributing.html
> which is more thorough.

Which is part of the new contributor docs. :)
Thank you both for the tips. Looks like these files are still using the legacy commands:
As discussed with Vnicius on IRC he will get started to implement the new action commands in so that we can move out all Actions code into a separate module. By doing that he will start with the Null action which is easiest to implement, and while doing that also creating new testcases in Once done he will follow-up with key and mouse actions.

Vinicius, please let me know if you have further questions or if you are stuck with something.
Henrik, I'm reading the template class you've mentioned here: 

I've noticed this class receives a 'Session' object as parameter, and its methods use 'send_session_command', which is defined in:

Ok, so when the perform() method is called in there, it receives an actions list as a parameter, which are added as values to the "actions" keys in the body dictionary. This is used to call the 'send_session_command' method, which in turn forms an url with the parameters received.

Where is this url being used? I imagine it's a way of communicating with the mentioned actions endpoints, or am I mislead?
Flags: needinfo?(hskupin)
So you don't have to go that in-depth with the wdclient given that it makes use of an extra layer, what Marionette client doesn't have to. When you look at everything above `send_session_command()` that will be fine. Note that when you check the Marionette unit tests, and Marionette specifically there is a `_send_message()` command, which will also receive the action details via its body argument.

Maybe lets do the following:

1) Create a new module for Marionette client, and move all the existent action code in there. Make sure to get all the above mentioned tests working, and if rename to 

This will make sure that consumers of Marionette client won't have to immediately change their code but only have to update the class names, and we can keep backward-compatibility for a couple of Firefox releases.

2) Create a new module for Marionette client for the new WebDriver conforming implementation. While doing so create new tests in

I think step 1 should be kinda straight forward, and you should be able to get this done yourself. Once done we can get started with step 2, and I will definitely help you with that.
Flags: needinfo?(hskupin)
Hi Vinicius, it's been a while since I heard from you. I would like to know if you are still interested to work on this bug, and if you need help to continue. Please let me know. Thanks.
Flags: needinfo?(viniciuscosta0197)
Hi Henrik. Sorry for being away for so much time. I've had some exams during the last two weeks and I've been unable to work on the bug during this period. But I've started looking into it again yesterday. I'm still interested in working on the bug.

Just one quick question: where should be located? I will try to have that working by tomorrow, so I can contact you via IRC to work on step 2.
Flags: needinfo?(viniciuscosta0197)
Hey, that is great to hear! I'm looking forward to continue mentoring you on that bug.

Just place it right next to the module in the driver directory. Even I mentioned above that we want to keep that file for allowing consumers to change we might want to remove it before landing the patch. We will have to see how much code is changing and if it is worth keeping this ballast, especially if there no big use anymore.
I've had some problems with my local build of Firefox. Hopefully it will be fixed by tomorrow, and I will be able to test my local changes.
I've fixed my build and moved the Actions class (and the necessary MouseButton class) to a separate file. I've just tested it with and it's working fine. Henrik, should I change all files currently using this class to the new file?
Flags: needinfo?(hskupin)
Yes, that's what you should do. It will make it clear that this is for the legacy actions.
Flags: needinfo?(hskupin)
Summary: Remove legacy actions commands from Marionette → Replace legacy actions with WebDriver action commands in Marionette client
Hey Vinícius. I wanted to check back how you are, and if it is something you still want to work on. Meanwhile the removal of the old API blocks us in some areas, and it would be nice to see it removed in the near future. 

Please let me know what you think, and if this bug is too hard, we can certainly find an easier one for you. That's totally fine.
Flags: needinfo?(viniciuscosta0197)
Hi Henrik. At the moment I'm trying to update the most simple action commands (click, key_down) in to use the WebDriver conforming ones (since those have equivalents in

Like, for example, has the following code for click:

>    def click(self, element=None, button=0):
>        """Queue a click with the specified button.
>       If an element is given, move the pointer to that element first,
>        otherwise click current pointer coordinates.
>        :param element: Optional element to click.
>        :param button: Integer representing pointer button to perform action
>                       with. Default: 0, which represents main device button.
>        """
>        if element:
>            self.pointer_move(0, 0, origin=element)
>        return self.pointer_down(button).pointer_up(button)

While the legacy code uses

>    def click(self, element, button=MouseButton.LEFT, count=1):
>        '''
>        Performs a click with additional parameters to allow for double clicking,
>        right click, middle click, etc.
>        :param element: The element to click.
>        :param button: The mouse button to click (indexed from 0, left to right).
>        :param count: Optional, the count of clicks to synthesize (for double
>                      click events).
>        '''
>        el =
>        self.action_chain.append(['click', el, button, count])
>        return self

I'm trying to learn how WebDriver does the same action differently. I've noticed it uses key and pointer actions, for instance. This is taking some time to learn (since I'm only working during weekends, I end up forgetting some parts of the code and have to read it again).

Although it's a little bit hard, I'm learning a lot with it. But yes, I'm making a slow progress. If it's urgent for the team, that's no problem if you find it better for me to work on something else.

Meanwhile, I will try running some tests later today to see if I can make the click action run correctly. I think that writing the other actions will be easier after that.
Flags: needinfo?(viniciuscosta0197)
I have a question regarding the 'perform' action here In the legacy code, the perform action is executed by sending a message to the Marionette server with the Action Chain. We will continue using this? (the WebDriver code uses a session. Not entirely sure how that works yet, so I will look into it).
Flags: needinfo?(hskupin)
(In reply to Vinícius Costa from comment #17)
> Although it's a little bit hard, I'm learning a lot with it. But yes, I'm
> making a slow progress. If it's urgent for the team, that's no problem if
> you find it better for me to work on something else.

That's good to hear that such a challenging task isn't holding you off, and you enjoy learning from it! 

Regarding the timing, it would be indeed good to have this done better sooner than later. And I can understand how hard it is when you have to pick up the work from 6 or more days ago. So my proposal would be that we find something else for you, which is also challenging but doesn't hold of other projects. And sorry that this wasn't that clear earlier.
(In reply to Vinícius Costa from comment #18)
> tools/webdriver/webdriver/ In the legacy code, the perform
> action is executed by sending a message to the Marionette server with the
> Action Chain. We will continue using this? (the WebDriver code uses a
> session. Not entirely sure how that works yet, so I will look into it).

To also reply to this question... Yes, that is what we want to continue to do. The `perform` command in Marionette client would also call `send_message()` similar to the one from the legacy commands. That method is actually handling the session transparently for you, and as such we don't have to pass it into each and every method.
Flags: needinfo?(hskupin)
That's fine Henrik. It was a nice learning experience! So, if there's some other challenging and less urgent task for me to work on, I would gladly look into it. Like I said to you in e-mail, I'm only able to work during weekends until late November, then I will have more spare time. But I can start working on something until then.
Maybe lets chat first on IRC so you can show me the current status as a patch, and we can discuss what would be the best option? If you already made big progress, I actually don't want to pull it off from you.
All right. Maybe IRC will be difficult for me this week, but I will try to wrap up what I've been working on and send a patch this weekend (after I run some tests and make sure I'm not breaking anything). Either way I will keep in touch with you via e-mail, so there won't be long periods without you hearing from me.
Ok, I think I hit a wall with the perform action.

Based on webdriver, I'm trying to implement `key_up` the following way in Marionette client:

>    def _key_action(self, subtype, value):
>        self.action_chain.append({"type": subtype, "value": value})

>    def key_up(self, key_code):
>        self._key_action("keyUp", key_code)
>        return self

That's of course different from how these objects were stored in action_chain in legacy code. The `_send_message()` method in `perform()` expects a body like this to be sent to the marionette server:

> body = {"chain": self.action_chain, "nextId": self.current_id}

So, I'm stuck at how should I update this `perform()` function so it works with the new webdriver conforming code. I think the problem is that I don't understand why are actions stored like that, with "type" and "value" fields (what goes in these?). And what does the marionette driver server expects to receive in the body?
Flags: needinfo?(hskupin)
You will have to also change `perform()`. The command's name and body to be sent are different now. So the new endpoint is `WebDriver:PerformActions`. For how the body looks like you can as best check one of the Wd jobs on Treeherder ( The action tests are most likely part of Wd2. Click on one of those and then on `live_backing.log` in the panel below. Search for `WebDriver:PerformActions`. Then you will see how it looks like. Here an example:

> Marionette	TRACE	0 -> [0,3,"WebDriver:PerformActions",{"actions":[{"actions":[{"type":"keyUp","value":"a"}],"id":"keyboard_id","type":"key"}]}]

Let me know if there are still open questions. Thanks!
Flags: needinfo?(hskupin)
Hey Henrik. I've submitted a partial patch just so you can have a look if I'm in the right path. At the moment, I'm trying to udpate the .perform action so it uses the new actions endpoint that you said. I'm doing things similar to this other file :, but the main problem now is that I've updated the __init__ method in the Actions class so it receives input_id and action_type as arguments, and now the tests in are failing because they should only send only one argument.

Since the new endpoint expects to receive these new arguments (as I've seen in the `live_backing.log` you've mentioned), how should I instantiate the class in the tests so it receives these new parameters?

This is one example (in :
>    def setUp(self):
>        self.key_action = Actions(self.marionette)

>    def test_key_action_basic_input(self):
>        self.key_action.key_down("a").key_down("b").key_down("c").perform()
>        self.assertEqual(self.key_reporter_value, "abc")

This test obviously gives an error because I'm instantiating the Actions class with just one argument, when it should also receive input_id and action_type.
Flags: needinfo?(hskupin)
We will continue the conversation in phabricator to not have two independent discussion threads.
Flags: needinfo?(hskupin)
Assignee: nobody → viniciuscosta0197
Mentor: hskupin

Hi Vinícius. I haven't heard from you for a while, and wonder how things are. Will you be able to continue on this bug, or should I take it over? Thanks.

Flags: needinfo?(viniciuscosta0197)

Hi Henrik. I think you should take over the bug. I haven't worked actively on it for quite some time now, and I don't want this feature to be delayed even further because of that. Sorry for taking this long.

Flags: needinfo?(viniciuscosta0197)

Ok, thanks a lot. I will take it over to clean-up our code base before making element send keys using the action primitives.

Assignee: u602518 → hskupin
Blocks: 1418995
Priority: P3 → P1
Mentor: hskupin
Attachment #9022463 - Attachment is obsolete: true

We will first land the new Action API support via bug 1533786.

Assignee: hskupin → nobody
Priority: P1 → P3
No longer blocks: 1025040
No longer blocks: 1534702

The following patch is waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D15931 Bug 1354578 - Updated ActionSequence class to WebDriver.r=whimboo whimboo viniciuscosta: Disabled

:whimboo, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Flags: needinfo?(hskupin)
Attachment #9034982 - Attachment is obsolete: true

With bug 1683755 the legacy actions support got actually removed in Firefox 86. So I don't see that anything else still needs to be done here.

Closed: 2 years ago
Flags: needinfo?(hskupin)
Resolution: --- → FIXED
Summary: Replace legacy actions with WebDriver action commands in Marionette client → [meta] Replace legacy actions with WebDriver action commands in Marionette client
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.