Closed Bug 1431127 Opened 2 years ago Closed 2 years ago

Fix TypeError: panel._selectors.getPause

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(firefox59 verified, firefox60 verified)

VERIFIED FIXED
Firefox 60
Tracking Status
firefox59 --- verified
firefox60 --- verified

People

(Reporter: jlast, Assigned: jlast)

Details

Attachments

(1 file, 1 obsolete file)

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
Attached patch fix-pause.patch (obsolete) — Splinter Review
Attachment #8943293 - Flags: review?(jdescottes)
just verified that it continues to work in the old debugger.
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+
Assignee: nobody → jlaster
Status: NEW → ASSIGNED
Priority: -- → P3
Comment on attachment 8943293 [details] [diff] [review]
fix-pause.patch

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

Please use a proper commit message!
Attachment #8943293 - Attachment is obsolete: true
Attachment #8943709 - Flags: review?(jdescottes)
Attachment #8943709 - Flags: review?(jdescottes) → review+
Is the try run for a new version of the patch? Otherwise feel free to land this.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e42d6d64671
Fix TypeError: panel._selectors.getPause. r=jdescottes
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9e42d6d64671
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
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 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+
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+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.