1558506 - [about:addons] No context menu opened when triggered in the inline options ui XUL browser

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

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

In Firefox 69 triggering the context menu in the extensions options_ui embedded in about:addons doesn't open the context menu anymore.

The related tests defined in browser_ext_optionsPage_popups.js are still able to trigger the context menu (I'm pretty sure that the test is still working because it is currently triggering the event directly in the remote XUL browser, instead of BrowserTestUtils.synthesizeMouseAtCenter) and so we missed to notice the regression.

Bisecting the issue pointed to Bug 1505909, which makes sense given that in Bug 1505909 the context menu internals have been adapted to make them Fission-aware.

After digging a bit, it seems to me that the context menu is actually being triggered, but the event is being intercepted and handled by the ContextMenuChild instance related to the "about:addons" page that contains the options_ui browser XUL element, before the ContextMenuChild related to the options ui extension page had any chance to intercept and handle it.

e.g. right-clicking on a <a href="..."></a> DOM element part of an extension options page,
is intercepted and handled as an event targeting the options_ui browser XUL element, instead of the DOM element part of the extension options page.

To quickly double-check that this is actually the case, I briefly tried to make ContextMenuChild's handleEvent to early exit and not handling the event if the composedTarget is a XUL browser element embedded, and at that point the expected context menu has been triggered on the extension page's <a href="..."></a> DOM element.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Thanks for filing! Looking.

Comment 2

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

Attachment: test-webextpanel-contextmenu.xpi

Here is the test extension that I used to test the issue (and bisect it with mozregress)

The extension contains a simple HTML page with a link (on which I was right-clicking to trigger the context menu) used as an options page embedded in the about:addons details view (and also reused on the sidebar, browserAction popup and extension devtools panel).

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Thanks, rpl. I'm able to reproduce now, and have a solution en route. Much appreciated!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1558506 - Don't send a message for context menus when we know we don't want to open one. r?NeilDeakin

This fixes Bug 1558506 by making it so that the parent process ignores the context menu event
when right-clicking on the remote <browser> hosting the Add-on Options page. Before, we were
handling the event, stopping it from propagating and preventDefault'ing it, and then sending
a message to the parent that ultimately did nothing (since we knew that we didn't want to
display the context menu). Stopping propagation and preventDefault'ing meant that the event
was never fired in the Extension process for the options page.

With the parent process now returning early in the event that it knows that it doesn't want
to be the one to open the context menu, the underlying ContextMenuSpecialProcessChild can
handle the contextmenu event in the extension process, and do the right thing.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3876241cc1ec
Don't send a message for context menus when we know we don't want to open one. r=NeilDeakin

Comment 6

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 3876241cc1ec (Bug 1558506) for mochitest failrues at test_contextmenu_list.xul.

Backout: https://hg.mozilla.org/integration/autoland/rev/55d6d2853f295c1024c344bc696b68fa25f4b4c8

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=3876241cc1ecfbc452fdcc3b05c9df44eb8f579e&selectedJob=251727205

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=251727205&repo=autoland&lineNumber=18758

Comment 7

[Cristian Brindusan [:cbrindusan]]

There are also dt failures on browser_aboutdebugging_devtoolstoolbox_contextmenu.js
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=251733835&repo=autoland&lineNumber=1782

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1558506 - Don't preventDefault the contextmenu event in ContextMenuChild.jsm. r?NeilDeakin

In the content process case, preventing default stops the context menu event from being dispatched
within remote iframes, which is what causes bug 1558506.

In the parent process case, preventing default stops the nsXULPopupListener from opening the context
menu for us when we don't want ContextMenuParent to handle it, which we don't want to do.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff0381bf498fd3d0260df92d225d36e283378dde&selectedJob=251916759

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/053143993e49
Don't preventDefault the contextmenu event in ContextMenuChild.jsm. r=NeilDeakin

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/053143993e49

Comment 12

[Madalin Cotetiu]

Verified in Win7x64 and MacOS 10.14.1 using FF70.0a1 (20190710215049) and FF69.0b3 using the extension provided in comment 2.
I have installed the extension and have verified the context menu can be open when right clicking the link in both places it is displayed (the side bar and the options menu in about:addons).

Marking bug as verified fixed.