Closed
Bug 1118806
Opened 8 years ago
Closed 8 years ago
Handle uncaught promise rejections in the storage inspector
Categories
(DevTools :: Storage Inspector, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 38
People
(Reporter: ejpbruel, Assigned: ejpbruel)
References
Details
Attachments
(1 file, 1 obsolete file)
2.38 KB,
patch
|
pbro
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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 2•8 years ago
|
||
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+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/86ac3a19e456
Comment 4•8 years ago
|
||
Backed out for mochitest-dt orange. https://hg.mozilla.org/integration/fx-team/rev/8d3e410aa24a https://treeherder.mozilla.org/ui/logviewer.html#?job_id=1637841&repo=fx-team
Assignee | ||
Comment 5•8 years ago
|
||
For the next attempt, let's try this on try first: https://tbpl.mozilla.org/?tree=Try&rev=c6db05fe0011
Assignee | ||
Comment 7•8 years ago
|
||
Try push was green, but discovered another uncaught rejection, so here's a second push just to make sure: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=39880ae71804
Assignee | ||
Comment 8•8 years ago
|
||
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 9•8 years ago
|
||
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+
Assignee | ||
Comment 10•8 years ago
|
||
Here goes nothing. Nobody tell RyanVM where I live: https://hg.mozilla.org/integration/fx-team/rev/c9ff49636eef
Comment 11•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c9ff49636eef
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Updated•8 years ago
|
Target Milestone: Firefox 37 → Firefox 38
Updated•4 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•