Closed
Bug 1431127
Opened 8 years ago
Closed 7 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•8 years ago
|
||
Attachment #8943293 -
Flags: review?(jdescottes)
Assignee | ||
Comment 2•8 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•8 years ago
|
||
just verified that it continues to work in the old debugger.
Comment 4•8 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•8 years ago
|
Comment 5•8 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•8 years ago
|
||
Attachment #8943293 -
Attachment is obsolete: true
Attachment #8943709 -
Flags: review?(jdescottes)
Updated•8 years ago
|
Attachment #8943709 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
Is the try run for a new version of the patch? Otherwise feel free to land this.
Updated•7 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•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 11•7 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•7 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•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
Flags: qe-verify+
Comment 14•7 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•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•