Make use of "once" option for addEventListener

RESOLVED DUPLICATE of bug 1291320

Status

Testing
Marionette
--
major
RESOLVED DUPLICATE of bug 1291320
a year ago
9 months ago

People

(Reporter: whimboo, Unassigned, Mentored)

Tracking

({ateam-marionette-server})

Version 3
ateam-marionette-server
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=js])

With bug 1287706 we got support for one-time listeners which would not require an extra call to removeEventListener. I think it would be kinda handy in Marionette and would save us a couple of lines in the marionette-server.
This is potentially _very_ exciting seeing as Marionette has had tonnes of bugs around failing to remove event listeners.  Increasing the importance of this bug.
Severity: normal → major
OS: Unspecified → All
Hardware: Unspecified → All
I talked with Andreas on IRC and he is happy to mentor this bug. So what needs to be done. First, get familiar with Marionette (https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette) and run our existent unit tests for the marionette driver. If that works, make sure to read all about event handling and using addEventListener (https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener).

What we have to do on this bug is to change all of those calls, which definitely get called only once before removeEventListener() is called to unregister the formerly handler. Handlers which will be called multiple times should not be changed. All the instances of addEventListener can be found here:

https://dxr.mozilla.org/mozilla-central/search?q=path%3Atesting%2Fmarionette+addEventListener&redirect=true
Mentor: ato@mozilla.com
Keywords: ateam-marionette-server
Whiteboard: [lang=js]
I have been reading about marionette and feel that I understand how server, client and gecko are linked. So I feel I can contribute to this bug.
 
Coming to the actual bug, I am trying to debug the events in driver.js and trying to find if it is called once. But i can see that the listener for the 
    win.removeEventListener("load", listener);
in 
    GeckoDriver.prototype.newSession"
makes a check if ev.target is win.document so i think its safe to assume that we cannot refactor this event.

And also the listener for the
    win.removeEventListener("load", listener);
in
    GeckoDriver.prototype.addFrameCloseListener
looks similar to the above scenario

Now coming to the things i am finding difficult to work on. Next i thought of debugging the event in evaluate.js and I dont have a clue on how to actually trigger that code. I guess it is probably via client side python api's?
Flags: needinfo?(ato)
(In reply to Nishanth Mane [:nishu-tryinghard] from comment #3)
> I have been reading about marionette and feel that I understand how server,
> client and gecko are linked. So I feel I can contribute to this bug.
>  
> Coming to the actual bug, I am trying to debug the events in driver.js and
> trying to find if it is called once. But i can see that the listener for the 
>     win.removeEventListener("load", listener);
> in 
>     GeckoDriver.prototype.newSession"
> makes a check if ev.target is win.document so i think its safe to assume
> that we cannot refactor this event.
> 
> And also the listener for the
>     win.removeEventListener("load", listener);
> in
>     GeckoDriver.prototype.addFrameCloseListener
> looks similar to the above scenario
> 
> Now coming to the things i am finding difficult to work on. Next i thought
> of debugging the event in evaluate.js and I dont have a clue on how to
> actually trigger that code. I guess it is probably via client side python
> api's?

its win.addEventListener not win.removeEventListener
(In reply to Nishanth Mane [:nishu-tryinghard] from comment #3)
> Coming to the actual bug, I am trying to debug the events in driver.js and
> trying to find if it is called once. But i can see that the listener for the 
>     win.removeEventListener("load", listener);
> in 
>     GeckoDriver.prototype.newSession"
> makes a check if ev.target is win.document so i think its safe to assume
> that we cannot refactor this event.

We can probably not make this use {once: true}, no.

> And also the listener for the
>     win.removeEventListener("load", listener);
> in
>     GeckoDriver.prototype.addFrameCloseListener
> looks similar to the above scenario

Yes.

> Now coming to the things i am finding difficult to work on. Next i thought
> of debugging the event in evaluate.js and I dont have a clue on how to
> actually trigger that code. I guess it is probably via client side python
> api's?

self.marionette.execute_script

On another note, there are a number of cases in testing/marionette/listener.js where we can employ {once: true} that might be easier to tackle.
Flags: needinfo?(ato)
refresh() will be fixed as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1291320.  I believe this is the last remaining use of event listeners that can (easily) be replaced with {once: true}.

Thanks to nishu for making us aware of this!
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1291320
You need to log in before you can comment on or make changes to this bug.