1519808 - DOMWindowUtils.sendMouseEvent sometimes fires an event that targets the entire HTML document instead of the expected DOM Element

Status: RESOLVED INVALID

(Core :: DOM: Events, defect)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

This issue is a follow up of Bug 1518883.

As briefly described in Bug 1518883 comment 3, the intermittent test was triggering a "contextmenu" event on a link DOM element using BrowserTestUtils.synthesizeMouseAtCenter but the resulting DOM event from time to time was actually targeting the entire HTML document instead of the expected DOM element, resulting in the following ContextMenuChild.jsm error (and no contextmenu is going to be shown because of this error):

"TypeError: doc is null" {file: "resource:///actors/ContextMenuChild.jsm" line: 478}

The reason for the "doc is null" error is that the event.composedTarget is not a DOM element, but an HTML document, and so its event.composedTarget.ownerDocument property is actually null.

Interestingly, if the BrowserTestUtils.synthesizeMouseAtCenter is called after a small (arbitrary) delay, the resulting mouse event seems to be targeting the expected DOM element with an higher frequency (but it can still fail from time to time on slower builds, e.g. linux32-debug).

By digging into what is going differently when the BrowserTestUtils.synthesizeMouseAtCenter calls are failing or succeeding, I noticed that in both the cases the left and top positions computed internally by the AsyncUtilsContent.js "Test:SynthesizeMouse" message handler have the same values, and so DOMWindowUtils.sendMouseEvent is being called using the same parameters, but the calls have different outcomes.

I suspect that DOMWindowUtils.sendMouseEvent may be having some issues on identify what DOM element is at a give position in the options_ui extension page also because this extension page is one of the few cases in which we have a browser XUL element embedded inside another browser XUL (e.g. like the issue with the select popup positioning that we fixed in Bug 1390445).

I'm going to attach to this issue a test case that is able to trigger the issue consistently.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: trigger_Bug_1519808___options_ui_page_DOMWindowUtils_sendMouseEvent_issue.patch

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

I'm moving this bug out of the "Toolkit::Add-ons Manager" bugzilla component, because this is definitely not happening only on the remote XUL browser related to the extensions options_page (e.g. in Bug 1484789 it is making the test to fail intermittently on the browserAction popup).

Moving it to Core::DOM, as the issue seems to be happening because of an unexpected behaviors of the underlying DOMWindowUtils.sendMouseEvent calls.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Sounds like caller is missing to flush layout before trying to use the API.

Luca, does it help if you do something like
expectedTarget.offsetLeft; // this forces layout

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Olli Pettay [:smaug] (PTO-ish Feb 16-23) from comment #3)

Sounds like caller is missing to flush layout before trying to use the API.

Luca, does it help if you do something like
expectedTarget.offsetLeft; // this forces layout

Thanks Olli, this has been very helpful!

First I tried to use the approach suggested, by applying the following change to our openContextMenuInPopup test helper:

  let browser = await awaitExtensionPanel(extension);
  await ContentTask.spawn(browser, selector, targetSelector => {
    let el = content.document.querySelector(targetSelector);
    el.offsetLeft;
  });
  ...
  await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "mousedown", button: 2}, browser);
  await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "contextmenu"}, browser);

but it didn't seem to be enough, the test case getTargetElement_in_browserAction_popup was still able to trigger the TypeError: doc is null error (and make the browser_ext_menus_targetElement_extension.js test file to fail intermittently):

Nevertheless, your comment got me thinking about trying another approach on the same line: "awaiting on the promiseDocumentFlushed promise before triggering the mouse events", and so I tried to apply the following change to the openContextMenuInPopup test helper:

  let browser = await awaitExtensionPanel(extension);

  await browser.ownerGlobal.promiseDocumentFlushed(() => {});
  ....
  await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "mousedown", button: 2}, browser);
  await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "contextmenu"}, browser);

and this approach worked pretty well, I executed the test multiple times with --verify, and it has always been able to complete successfully without triggering the TypeError: doc is null error.

And I think that you are 100% right and the underlying issue is that we are triggering the mouse event before the layout has been flushed (after we have just added the remote browser XUL element for the extension browserAction popup).

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

>   await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "mousedown", button: 2}, browser);
>   await BrowserTestUtils.synthesizeMouseAtCenter(selector, {type: "contextmenu"}, browser);

If you do it in a test, please dispatch "mouseup" event between "mousedown" and "contextmenu" events because of emulating actual behavior.

On the other hand, sounds like that EventUtils should support do it automatically if synthesizeMouse() is called with "contextmenu".

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hey Luca, sounds like it's not really an event issue. Should I close this as invalid?

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)

Hey Luca, sounds like it's not really an event issue. Should I close this as invalid?

Sure, closing this as invalid sounds good to me too, this bug already served its purpose and it doesn't seem that there is anything to do from a DOMEvent point of view.

Thanks again for the help on figuring this out.