1159276 - Multiple context menu popups show up when right clicking on a breakpoint in sources pane

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: two-menus.png

It opens both the Open in New Tab / Copy as URL popup for the main source and the breakpoint context menu.  See screenshot.

STR:
Open a page (any page, but you could use https://bgrins.github.io/devtools-demos/debugger/debugger.html)
Open debugger and set a breakpoint
Right click on that breakpoint

Expected:
You see only the breakpoint context menu

Actual:
You see both the source context menu and the breakpoint context menu

Comment 1

[Mike Taylor [:miketaylr]]

Beautiful.

Comment 2

[Mike Taylor [:miketaylr]]

bgrins, does this reproduce 100%? I'm can't get double contextmenus in the latest Nightly: https://cloudup.com/ceAj_iw47-4

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Mike Taylor [:miketaylr] from comment #2)
> bgrins, does this reproduce 100%? I'm can't get double contextmenus in the
> latest Nightly: https://cloudup.com/ceAj_iw47-4

I'm seeing it on a local build / nightly on OSX and linux, but not on a local build in Windows.  So looks like it affects non-windows platforms.

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: context-menu-debugger.patch

Here's a potential solution.  The issue is that the debugger frontend is setting the `contextmenu` attribute on the breakpoint elements: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-panes.js#247.  AFAIK there isn't a way to stopPropagation when showing the menus via the attribute, so here is a way to check and see if anything has that attribute.

The alternative would be to change how those context menus are shown, by using addEventListener("contextmenu") on the breakpoint item.  I'm not super familiar with this code though.  James, any idea how easy that would be?  I can't seem to find the source of sourceItem.append from the link above.

Comment 5

[James Long (:jlongster)]

This looks like a valid solution. I wouldn't bother reworking how the context menus are shown honestly, because I have some ideas for reworking this part of the code in general. I don't know how hard it would be, but it might not work well with how the widgets are currently setup.

Comment 6

[James Long (:jlongster)]

Comment on attachment 8598719 [details] [diff] [review]
context-menu-debugger.patch

Review of attachment 8598719 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: context-menu-debugger-r=jlongster.patch

With test.  Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b9e4cca8a92

Comment 8

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/dd8c914eb1f2

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dd8c914eb1f2