Check click event target to decide whether to synthesize a dblclick

NEW
Unassigned

Status

enhancement
P2
normal
a year ago
a year ago

People

(Reporter: maja_zf, Unassigned)

Tracking

(Blocks 2 bugs)

Version 3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

The way we synthesize dblclick in Marionette actions right now (as implemented in Bug 1385476) doesn't match Firefox behaviour or the UIEvents spec because it does not check whether the first and second click are on the same target element [1].

From UIEvents spec: "A user agent MUST dispatch this event when the primary button of a pointing device is clicked twice over an element. The definition of a double click depends on the environment configuration, except that the event target MUST be the same between mousedown, mouseup, and dblclick."

[1] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/testing/marionette/event.js#66
Oh! That might be the reason then why we see the test failures (bug 1421878) across different tests, right? It sounds very unlikely that we would hit those conditions with this fix in place.
Reporter

Comment 2

a year ago
Not necessarily, because most of those tests are interacting with the same element. 

That makes me think of another problem -- I imagine the "click" state should reset whenever the document is loaded. I'm not sure that happens now.
That sounds reasonable too!
Reporter

Comment 4

a year ago
So the click-timer reset is in listener.js: https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/testing/marionette/listener.js#51-56

How does listener work again? 
Is listener the same thing as the "frame script" that gets injected to the content frame? <--- (That question likely makes no sense.) Does that happen only once per session? Where would be a good place to call resetClick on document load or unload? 

Sorry, I'm out of my element when it comes to listener.js :)
Flags: needinfo?(ato)
(In reply to Maja Frydrychowicz (:maja_zf) from comment #4)

> How does listener work again?  Is listener the same thing as the
> "frame script" that gets injected to the content frame? <--- (That
> question likely makes no sense.)

Your question makes perfect sense!  Yes,
testing/marionette/listener.js is a frame script as described in
[1].  Code that needs direct access to content lives in frame
scripts that are loaded by the frame’s message manager.  Because
content browsers run in a different process to our chrome code, we
use IPC messages over the message manager API to communciate with
frame scripts.

> Does that happen only once per session?

We load listener.js once per frame, so it is not tied to the
Marionette session at all.  The message listeners in the frame
script are currently removed when the session is deleted, but this
will change with the forthcoming window tracking changes in [2].

Because we use the aAllowDelayedLoad argument to
nsIFrameScriptLoader.loadFrameScript, listener.js is also
automatically loaded into any new frames that appear in content
subsequent to the invocation of loadFrameScript.  This is why we
don’t have to call loadFrameScript again for i.e. <iframe>s.

> Where would be a good place to call resetClick on document load or
> unload?

I would probably say you could just add an event listener on the DOM
unload event.  Or would this not work?

I hope this helps!

  [1] https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Message_Manager/Message_manager_overview
  [2] https://bugzil.la/marionette-window-tracking
Blocks: webdriver
Flags: needinfo?(ato)
Priority: -- → P2
Summary: (actions) Check click event target to decide whether to synthesize a dblclick → Check click event target to decide whether to synthesize a dblclick
Reporter

Updated

a year ago
Duplicate of this bug: 1448487

Comment 7

a year ago
Is this annoying bug going to be fixed in Firefox 60? Otherwise, it makes it into the ESR, and will be around in all tests until Mid 2019.
You need to log in before you can comment on or make changes to this bug.