Closed
Bug 1275078
Opened 8 years ago
Closed 7 years ago
Merge toolbox-init.js from Positron branch
Categories
(DevTools :: General, enhancement)
Tracking
(firefox54 fixed)
RESOLVED
FIXED
Firefox 54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: myk, Assigned: jryans)
References
Details
Attachments
(2 files)
After bug 1268134 is resolved, we should merge toolbox-init.js from Positron again to address these issues, per <https://github.com/mozilla/positron/pull/50>: 1. The script tests `if (iframe)` after calling `iframe.QueryInterface()`, but that call should throw an exception if *iframe* is falsy, so the conditional will never be false. 2. After fixing issue #1 by testing *iframe* before calling `iframe.QueryInterface()`, if *iframe* is falsy, then *toolboxOpened* is never defined, so the `toolboxOpened.catch()` call below will fail. 3. The promise chain will reach the `toolboxOpened.then()` function even if there was an error opening the toolbox, since it gets attached to the chain after `toolboxOpened.catch()`, which doesn't re-throw the error.
Assignee | ||
Updated•8 years ago
|
Severity: normal → enhancement
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Summary: merge toolbox-init.js from Positron branch → Merge toolbox-init.js from Positron branch
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d244415582fe01f3137a7ff0cea946ea3849457
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=636601e4ebda4f0e56a90b577dbea1c5fa48a5d1
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8840040 [details] Bug 1275078 - Fix ESLint issues in toolbox and target files. https://reviewboard.mozilla.org/r/114594/#review116344 ::: devtools/client/framework/toolbox-options.js:415 (Diff revision 2) > - this._origJavascriptEnabled = !response.javascriptEnabled; > + this._origJavascriptEnabled = !response.javascriptEnabled; > - this.disableJSNode.checked = this._origJavascriptEnabled; > + this.disableJSNode.checked = this._origJavascriptEnabled; > - this.disableJSNode.addEventListener("click", > - this._disableJSClicked); > - }); > + this.disableJSNode.addEventListener("click", this._disableJSClicked); > + } else { > + // Hide the checkbox and label > + this.disableJSNode.parentNode.style = "display: none;"; this.disabledJSNode.parentNode.style.display = "none"; ?
Attachment #8840040 -
Flags: review?(poirot.alex) → review+
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8840041 [details] Bug 1275078 - toolbox-init.js error handling tweaks from Positron. https://reviewboard.mozilla.org/r/114596/#review116346
Attachment #8840041 -
Flags: review?(poirot.alex) → review+
Assignee | ||
Comment 9•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8840040 [details] Bug 1275078 - Fix ESLint issues in toolbox and target files. https://reviewboard.mozilla.org/r/114594/#review116344 > this.disabledJSNode.parentNode.style.display = "none"; ? Thanks, fixed.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f66f556b7893 Fix ESLint issues in toolbox and target files. r=ochameau https://hg.mozilla.org/integration/autoland/rev/22c5f7ea3629 toolbox-init.js error handling tweaks from Positron. r=ochameau
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f66f556b7893 https://hg.mozilla.org/mozilla-central/rev/22c5f7ea3629
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•