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
10 months ago
6 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

10 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

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

Comment 1

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

Comment 2

10 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

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

Comment 4

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

Comment 5

10 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

10 months ago
Duplicate of this bug: 1301955

Comment 7

10 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

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

Comment 8

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

Comment 9

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

Comment 10

10 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

10 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

10 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

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

Updated

10 months ago
Keywords: checkin-needed

Comment 14

10 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

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b20ba15bc07d
Status: ASSIGNED → RESOLVED
Last Resolved: 10 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.