Closed
Bug 1431127
Opened 6 years ago
Closed 6 years ago
Fix TypeError: panel._selectors.getPause
Categories
(DevTools :: Debugger, defect, P3)
DevTools
Debugger
Tracking
(firefox59 verified, firefox60 verified)
VERIFIED
FIXED
Firefox 60
People
(Reporter: jlast, Assigned: jlast)
Details
Attachments
(1 file, 1 obsolete file)
2.63 KB,
patch
|
jdescottes
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
We started getting this error when we refactored the pause reducer to use isPaused
> Full message: TypeError: panel._selectors.getPause is not a function
> Full stack: setupThreadListeners@chrome://devtools/content/framework/toolbox-process-window.js:167:19
> bindToolboxHandlers/<@chrome://devtools/content/framework/toolbox-process-window.js:161:7
> process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
> walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
> Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
> schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
> completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
> promise callback*completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:698:7
> Promise.resolve/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:512:36
> Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:390:5
> Promise.resolve@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:512:10
> onLoad@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:1698:7
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #8943293 -
Flags: review?(jdescottes)
Assignee | ||
Comment 2•6 years ago
|
||
Here is what it looks like with the bug: https://shipusercontent.com/2e8ed975f75cf1eb3d3571c8d765cdb7/Screen%20Shot%202018-01-17%20at%2010.50.03%20AM.png Here is what it looks like with the fix: https://shipusercontent.com/e7e0f7518edff96c9f36e60cd86d8a02/Screen%20Shot%202018-01-17%20at%2010.47.35%20AM.png
Assignee | ||
Comment 3•6 years ago
|
||
just verified that it continues to work in the old debugger.
Comment 4•6 years ago
|
||
Comment on attachment 8943293 [details] [diff] [review] fix-pause.patch Review of attachment 8943293 [details] [diff] [review]: ----------------------------------------------------------------- Good fix for new frontend. Won't work with old frontend, but not a regression and non blocking. ::: devtools/client/framework/toolbox-process-window.js @@ +163,5 @@ > } > } > > function setupThreadListeners(panel) { > + updateBadgeText(panel.isPaused()); (Since you mentioned it :)) It does not work with the old frontend. I think you tested with the old frontend in Firefox, but not the old frontend in the Browser Toolbox. To enable it you need to : - open BT - select other panel than debugger - close BT - open BT - go preferences - disable new frontend - switch to debugger It was already not working previously. We could check if isPaused is available and is a function. Not mandatory IMO.
Attachment #8943293 -
Flags: review?(jdescottes) → review+
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Comment on attachment 8943293 [details] [diff] [review] fix-pause.patch Review of attachment 8943293 [details] [diff] [review]: ----------------------------------------------------------------- Please use a proper commit message!
Assignee | ||
Comment 6•6 years ago
|
||
Attachment #8943293 -
Attachment is obsolete: true
Attachment #8943709 -
Flags: review?(jdescottes)
Updated•6 years ago
|
Attachment #8943709 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 7•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=920efa940a87284db384820397074189bdda1999
Comment 8•6 years ago
|
||
Is the try run for a new version of the patch? Otherwise feel free to land this.
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9e42d6d64671 Fix TypeError: panel._selectors.getPause. r=jdescottes
Keywords: checkin-needed
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9e42d6d64671
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 11•6 years ago
|
||
Comment on attachment 8943709 [details] [diff] [review] fix-pause-2.patch Minor cosmetic feature, but allows to get rid of a JS error. Approval Request Comment [Feature/Bug causing the regression]:N/A (a previous debugger bundle update from https://bugzilla.mozilla.org/show_bug.cgi?id=debugger-bundle-updates) [User impact if declined]: MacOS users developing using the addon or browser toolbox (webextension developers for instance) will not see the red "start/pause" marker on the Firefox instance icon. [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: yes. - use MacOS - go to about:debugging#addons - enable addon debugging if not checked already - click on debug link of any addon - toolbox opens as new app ER: the app icon (visible eg. in the dock) shows a "►" character [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: minor javascript change [String changes made/needed]: none
Attachment #8943709 -
Flags: approval-mozilla-beta?
Comment 12•6 years ago
|
||
Comment on attachment 8943709 [details] [diff] [review] fix-pause-2.patch Useful fix for Mac developers, let's take this for the 59 beta 4 build.
Attachment #8943709 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 13•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/631af655e23e
Updated•6 years ago
|
Flags: qe-verify+
Comment 14•6 years ago
|
||
This issue is verified fixed using Firefox 60.0a1 (BuildId:20180206100151) and Firefox 59.0b7 (BuildId:20180205211730) on macOS 10.12.6.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•