Context menu on debugger editor doesn't open after Bug 1295213

VERIFIED FIXED in Firefox 51

Status

()

Firefox
Developer Tools: Debugger
P1
normal
VERIFIED FIXED
9 months ago
5 months ago

People

(Reporter: bgrins, Assigned: jlongster)

Tracking

({regression})

51 Branch
Firefox 51
regression
Points:
---

Firefox Tracking Flags

(firefox48 unaffected, firefox49 unaffected, firefox50 unaffected, firefox51 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 months ago
Haven't done a full regression window, but since it's working for me in nightly but broken in a local build it must be due to Bug 1295213.

STR:
Open a page
Open debugger
Right click in editor

Expected:
Context menu opens

Actual:
No menu, this error in browser toolbox:

TypeError: popup is null: SourcesView.prototype<._onEditorContextMenuOpen@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/debugger/content/views/sources-view.js:1117:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:191:13
Editor.prototype._setup/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/sourceeditor/editor.js:401:7
EventListener.handleEvent*Editor.prototype._setup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/sourceeditor/editor.js:389:5
EventListener.handleEvent*Editor.prototype.appendTo@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/sourceeditor/editor.js:309:5
DebuggerView._initializeEditor@chrome://devtools/content/debugger/debugger-view.js:311:5
DebuggerView.initialize@chrome://devtools/content/debugger/debugger-view.js:66:5
DebuggerController.startupDebugger<@chrome://devtools/content/debugger/debugger-controller.js:234:11
TaskImpl.prototype._run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:273:3
createAsyncFunction/asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:247:14
DebuggerPanel.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/debugger/panel.js:51:19
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:454:5
DebuggerPanel.prototype.open@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/debugger/panel.js:50:12
Toolbox.prototype.loadTool/onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:1292:19
(Reporter)

Updated

9 months ago
Priority: -- → P1
(Reporter)

Comment 1

9 months ago
James, can you confirm this issue?
Flags: needinfo?(jlong)
(Assignee)

Comment 2

9 months ago
I can confirm. I reverted that bug and it's fixed. 

I think the fix is easy, and I'll have a patch in a second
Flags: needinfo?(jlong)
(Assignee)

Comment 3

9 months ago
Created attachment 8789932 [details] [diff] [review]
1301788.patch
Assignee: nobody → jlong
Status: NEW → ASSIGNED
Attachment #8789932 - Flags: review?(bgrinstead)
(Assignee)

Comment 4

9 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=240ff924d1de
(Reporter)

Comment 5

9 months ago
Comment on attachment 8789932 [details] [diff] [review]
1301788.patch

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

Works for me with a green try, thanks.  Can you also update the commit message to explain what it's doing?
Attachment #8789932 - Flags: review?(bgrinstead) → review+

Updated

9 months ago
Duplicate of this bug: 1301955

Comment 7

8 months ago
Last good revision: a3a5d19b2d0539c66e787e0f2f86c539fb1f6fd8
First bad revision: 3a44273280cf5dd2a761c91fb9dac626c9d0700d
Pushlog: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=a3a5d19b2d0539c66e787e0f2f86c539fb1f6fd8&tochange=3a44273280cf5dd2a761c91fb9dac626c9d0700d

INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1295213 (Same bug as Brian mentioned above)
status-firefox48: --- → unaffected
status-firefox49: --- → unaffected
status-firefox50: --- → unaffected
status-firefox51: --- → affected

Updated

8 months ago
Version: unspecified → 51 Branch
(Reporter)

Comment 8

8 months ago
New try push (previous didn't run any jobs): https://treeherder.mozilla.org/#/jobs?repo=try&revision=e64ef040efa5

Comment 9

8 months ago
I will retest it when the build is complete.

Comment 10

8 months ago
I tested this issue on windows XP and Ubuntu 16.04 32-bits with above build (comment 8),  and it is fixed. The context menu opens when user right clicks on debugger's editor.
(Reporter)

Comment 11

8 months ago
(In reply to Abe - QA from comment #10)
> I tested this issue on windows XP and Ubuntu 16.04 32-bits with above build
> (comment 8),  and it is fixed. The context menu opens when user right clicks
> on debugger's editor.

Thanks!  James, can you update the commit message as Comment 5 and format this as a proper patch and re-upload the patch for checkin or push it after the change?  The metadata at the top here: https://bug1301788.bmoattachments.org/attachment.cgi?id=8789932 looks different than usual with the 'commit' field and extra spacing before the message.
Flags: needinfo?(jlong)
(Assignee)

Comment 12

8 months ago
Certainly. I don't think I formatted it for hg, which is a different command, sorry. I will update the patch and push it.
Flags: needinfo?(jlong)
(Assignee)

Comment 13

8 months ago
Created attachment 8790477 [details] [diff] [review]
1301788.patch
Attachment #8789932 - Attachment is obsolete: true
(Assignee)

Updated

8 months ago
Keywords: checkin-needed

Comment 14

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/b20ba15bc07d
Use context menu from correct window in source editor. r=bgrins
Keywords: checkin-needed

Comment 15

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b20ba15bc07d
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
I have to reproduced this bug with Nightly 51.0a1 (2016-09-09) on Windows 7 64 bit;

The Bug's fix is verified on beta

Build ID 	20170105155013
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

[Testday-20170106]
(In reply to Md.Tarikul Islam Oashi from comment #16)
> I have to reproduced this bug with Nightly 51.0a1 (2016-09-09) on Windows 7
> 64 bit;
> 
> The Bug's fix is verified on beta
> 
> Build ID 	20170105155013
> User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101
> Firefox/51.0
> 
> [Testday-20170106]

Thanks!
Status: RESOLVED → VERIFIED
status-firefox51: fixed → verified
You need to log in before you can comment on or make changes to this bug.