Last Comment Bug 1301788 - Context menu on debugger editor doesn't open after Bug 1295213
: Context menu on debugger editor doesn't open after Bug 1295213
Status: VERIFIED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: 51 Branch
: Unspecified Unspecified
P1 normal (vote)
: Firefox 51
Assigned To: James Long (:jlongster)
:
: Jason Laster [:jlast]
Mentors:
: 1301955 (view as bug list)
Depends on:
Blocks: 1295213
  Show dependency treegraph
 
Reported: 2016-09-09 14:35 PDT by Brian Grinstead [:bgrins]
Modified: 2017-01-09 02:05 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
unaffected
verified


Attachments
1301788.patch (2.26 KB, patch)
2016-09-09 14:57 PDT, James Long (:jlongster)
bgrinstead: review+
Details | Diff | Splinter Review
1301788.patch (2.16 KB, patch)
2016-09-12 15:25 PDT, James Long (:jlongster)
no flags Details | Diff | Splinter Review

Description User image Brian Grinstead [:bgrins] 2016-09-09 14:35:19 PDT
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
Comment 1 User image Brian Grinstead [:bgrins] 2016-09-09 14:38:23 PDT
James, can you confirm this issue?
Comment 2 User image James Long (:jlongster) 2016-09-09 14:53:35 PDT
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
Comment 3 User image James Long (:jlongster) 2016-09-09 14:57:26 PDT
Created attachment 8789932 [details] [diff] [review]
1301788.patch
Comment 5 User image Brian Grinstead [:bgrins] 2016-09-09 15:01:39 PDT
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?
Comment 6 User image Alice0775 White 2016-09-11 07:41:22 PDT
*** Bug 1301955 has been marked as a duplicate of this bug. ***
Comment 7 User image Abe - QA (:Abe_LV) 2016-09-12 11:39:59 PDT
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)
Comment 8 User image Brian Grinstead [:bgrins] 2016-09-12 11:52:50 PDT
New try push (previous didn't run any jobs): https://treeherder.mozilla.org/#/jobs?repo=try&revision=e64ef040efa5
Comment 9 User image Abe - QA (:Abe_LV) 2016-09-12 12:07:58 PDT
I will retest it when the build is complete.
Comment 10 User image Abe - QA (:Abe_LV) 2016-09-12 13:56:01 PDT
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.
Comment 11 User image Brian Grinstead [:bgrins] 2016-09-12 14:01:32 PDT
(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.
Comment 12 User image James Long (:jlongster) 2016-09-12 14:21:20 PDT
Certainly. I don't think I formatted it for hg, which is a different command, sorry. I will update the patch and push it.
Comment 13 User image James Long (:jlongster) 2016-09-12 15:25:27 PDT
Created attachment 8790477 [details] [diff] [review]
1301788.patch
Comment 14 User image Pulsebot 2016-09-12 18:52:17 PDT
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
Comment 15 User image Carsten Book [:Tomcat] 2016-09-13 03:03:23 PDT
https://hg.mozilla.org/mozilla-central/rev/b20ba15bc07d
Comment 16 User image Md.Tarikul Islam Oashi 2017-01-06 08:50:57 PST
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]
Comment 17 User image Bogdan Maris, QA [:bogdan_maris] 2017-01-09 02:05:20 PST
(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!

Note You need to log in before you can comment on or make changes to this bug.