Closed Bug 1159276 Opened 7 years ago Closed 7 years ago

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

Categories

(DevTools :: Debugger, defect)

Unspecified
macOS
defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(2 files, 1 obsolete file)

Attached image 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
Whiteboard: [devedition-40][difficulty=easy]
Beautiful.
bgrins, does this reproduce 100%? I'm can't get double contextmenus in the latest Nightly: https://cloudup.com/ceAj_iw47-4
(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.
Flags: needinfo?(bgrinstead)
OS: Unspecified → Mac OS X
Attached patch context-menu-debugger.patch (obsolete) — Splinter Review
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.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Flags: needinfo?(jlong)
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.
Flags: needinfo?(jlong)
Comment on attachment 8598719 [details] [diff] [review]
context-menu-debugger.patch

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

This looks good to me
Attachment #8598719 - Flags: review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/dd8c914eb1f2
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
https://hg.mozilla.org/mozilla-central/rev/dd8c914eb1f2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.