Closed Bug 1591140 Opened 9 months ago Closed 8 months ago

The context menu is shown in the inspector when right clicking a DOM node

Categories

(DevTools :: Inspector, defect, P3)

71 Branch
defect

Tracking

(firefox-esr68 unaffected, firefox70 unaffected, firefox71 fixed, firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- fixed
firefox72 --- fixed

People

(Reporter: peter, Assigned: jdescottes)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [dt-q])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

  1. Go to about:config and prevent JS from generally disabling the context menu via "dom.event.contextmenu.enabled" (set to non-standard "false").
  2. Now open up the inspector (developer tools, CMD+I) and right click any DOM node.

Actual results:

Notice how the document's (for the lack of a better word) context menu opens up in addition to the one the inspector provides - even lying above the latter.
This is super anyoing, to say the least.

Expected results:

Up until a couple of days, the inspector's menu was the only menu shown when right clicking a DOM node, regardless of the value of the "dom.event.contextmenu.enabled" setting, which I haven't tinkered with in months.

The, imho, correct behaviour should be to ignore the setting within the context of the developer tools and never show the document's context menu ... just like it used to be.

Component: Untriaged → Inspector
Product: Firefox → DevTools

Thank you for filing. I can reproduce with the dom.event.contextmenu.enabled pref set to false. I'll attempt a regression.

Priority: -- → P3
Whiteboard: [dt-q]

First attempt at mozregression narrowed the window to something that landed between October 9th and October 30th, but was otherwise inconclusive. I'm trying again.

mozregression identified Bug 1581925 as the source. Nika, can you take a look?

Flags: needinfo?(nika)
Regressed by: 1581925

I am guessing that the reason for this behaviour is that devtools uses context menu events in order to control the context menu, like normal web content. Before the changes in bug 1581925, we were not running the context menu for the toolbox, so it was always allowed to control menus. However, not that we treat it more like a usual content browser, the content code is running, meaning it is subject to the same requirements as web content.

We might be able to fix this by not overriding the defaultPrevented behaviour when the event is fired in a chrome-privileged document here: https://searchfox.org/mozilla-central/rev/ce02064d8afc8673cef83c92896ee873bd35e7ae/browser/actors/ContextMenuChild.jsm#542. That being said, I don't know if this about:config setup is one we intend to support.

Flags: needinfo?(nika) → needinfo?(jdescottes)

I can confirm we use contextmenu events in many places in DevTools. I actually had to preventDefault on such events before we switched the toolbox to a <browser> element in https://hg.mozilla.org/mozilla-central/rev/7d309560313a, but of course with this setting it no longer works.

This preference is quite old but still seems supported in enterprise policies, so I assume we want to keep it. Let's try to skip it for DevTools only as you suggested.

Flags: needinfo?(jdescottes)
Status: UNCONFIRMED → NEW
Ever confirmed: true

@gl can you take a look at the DevTools test added here?
@nika @mconley I used this.browsingContext.docShell.isContent here to rule out chrome-privileged documents. Do you know if there is better way to check if the current document is chrome-privileged or not?

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e5ca446ceff7
Allow chrome privileged documents to preventDefault contextmenu events regardless of dom.event.contextmenu.enabled r=gl,nika,mconley
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72
Assignee: nobody → jdescottes

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(jdescottes)
Flags: in-testsuite+

Let's check with @mconley first, the fix is small but it's on a very central piece of the context menu code (and not my usual area).
Mike, do you think it's ok to uplift as is, or should it bake a bit more on Nightly?
(also considering it only impacts DevTools users with dom.event.contextmenu.enabled set to false)

Flags: needinfo?(jdescottes) → needinfo?(mconley)

I think this can be safely uplifted.

Flags: needinfo?(mconley)

Thanks for the feedback, let's request an uplift then.

Comment on attachment 9105641 [details]
Bug 1591140 - Allow chrome privileged documents to preventDefault contextmenu events regardless of dom.event.contextmenu.enabled

Beta/Release Uplift Approval Request

  • User impact if declined: Users with the pref dom.event.contextmenu.enabled set to false cannot use context menus in DevTools (the regular context menu is overlayed on top of the DevTools one).
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small javascript fix, tested in automation.
  • String changes made/needed:
Attachment #9105641 - Flags: approval-mozilla-beta?

Comment on attachment 9105641 [details]
Bug 1591140 - Allow chrome privileged documents to preventDefault contextmenu events regardless of dom.event.contextmenu.enabled

Low risk, has tests, uplift approved for 71 beta 9, thanks.

Attachment #9105641 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.