Closed Bug 1118806 Opened 8 years ago Closed 8 years ago

Handle uncaught promise rejections in the storage inspector


(DevTools :: Storage Inspector, defect)

Not set


(Not tracked)

Firefox 38


(Reporter: ejpbruel, Assigned: ejpbruel)




(1 file, 1 obsolete file)

Attempting to replace the deprecated-sync-thenables with Promise.jsm promises in protocol.js (bug 1096490) uncovered some uncaught promise rejections in the inspector. These need to be fixed before I can land the former.
Blocks: 1096490
I don't know who owns the storage inspector so flagging pbrosset for review is a guess.

Patrick, if you're not the right person for this feel free to unassign yourself. Or even better, assign the right person instead :-)
Attachment #8545312 - Flags: review?(pbrosset)
Comment on attachment 8545312 [details] [diff] [review]
Handle uncaught promise rejections

Review of attachment 8545312 [details] [diff] [review]:

Simple enough change for R+ even if I don't own this panel.
Attachment #8545312 - Flags: review?(pbrosset) → review+
For the next attempt, let's try this on try first:
Try push was green, but discovered another uncaught rejection, so here's a second push just to make sure:
Try push looks green. Rerequesting review for the additional changes.

Apparently I accidentally squashed two commits, so there are also some style inspector changes in this patch. It's less than 3 lines though, so easiest would be to just land it as one patch.
Attachment #8545312 - Attachment is obsolete: true
Attachment #8548018 - Flags: review?(pbrosset)
Comment on attachment 8548018 [details] [diff] [review]
Handle uncaught promise rejections

Review of attachment 8548018 [details] [diff] [review]:

r=me with a comment added

::: browser/devtools/styleinspector/style-inspector-overlays.js
@@ +183,5 @@
>    _hideCurrent: function() {
>      if (this.highlighterShown) {
>        this._getHighlighter(this.highlighterShown).then(highlighter => {
> +        let promise = highlighter.hide();
> +        if (promise) {

I think a comment is required before this condition. It will be otherwise hard to future readers to understand why hide sometimes returns a promise and sometimes not.
Attachment #8548018 - Flags: review?(pbrosset) → review+
Here goes nothing. Nobody tell RyanVM where I live:
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Target Milestone: Firefox 37 → Firefox 38
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.