Closed
Bug 1106079
Opened 9 years ago
Closed 9 years ago
Style Editor broken on XUL windows
Categories
(DevTools :: Style Editor, defect)
DevTools
Style Editor
Tracking
(firefox36 fixed, firefox37 fixed)
RESOLVED
FIXED
Firefox 37
People
(Reporter: pbro, Assigned: pbro)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
3.13 KB,
patch
|
pbro
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
STR: - Open "about:config" - Open the toolbox - Switch to the style-editor ER: everything should work fine AR: the editor is broken (doesn't show any stylesheets) and the following error is thrown: console.error: Message: Error: Custom SelectorHighlighterhighlighter cannot be created in a XUL window Stack: exports.CustomHighlighterActor<.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/highlighter.js:334:1 constructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js:146:23 exports.InspectorActor<.getHighlighterByType<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:3176:14 actorProto/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1004:19 DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1490:15 LocalDebuggerTransport.prototype.send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:545:11 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14 exports.CustomHighlighterActor<.initialize@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/highlighter.js:334:1 constructor@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/core/heritage.js:146:23 exports.InspectorActor<.getHighlighterByType<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:3176:14 actorProto/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1004:19 DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1490:15 LocalDebuggerTransport.prototype.send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:545:11 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14 makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14 DBG-SERVER: Packet 44 sent from "conn2.inspectorActor23" DBG-SERVER: Received packet 44: { "from": "conn2.inspectorActor23", "error": "unknownError", "message": "Error: Custom SelectorHighlighterhighlighter cannot be created in a XUL window" } console.error: Protocol error (unknownError): Error: Custom SelectorHighlighterhighlighter cannot be created in a XUL window
Assignee | ||
Comment 1•9 years ago
|
||
This is a problem not only with about:config but of course with the browser-toolbox because it inspects the browser window which is a XUL window. This error is related to the highlighter. The new highlighter is hosted as a native anonymous element in the canvasFrame. XUL windows do not have canvasFrames, and so any attempt to create a highlighter in a XUL window throws an exception. This was expected. What is not expected though is that this completely broke the style-editor. The long term solution is bug 1094959. The short term solution is to make the style editor handle the exception nicely and just disable the selector highlighting feature for XUL windows.
Updated•9 years ago
|
Keywords: regression
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
I have a fix for this already, just working on a test right now. Hopefully this can make it before the cut. If not, we can uplift it later. The regression is only in nightly at this stage as the new highlighter landed during this release cycle.
Comment hidden (obsolete) |
Assignee | ||
Comment 4•9 years ago
|
||
Forgot to add the test to source control.
Attachment #8530303 -
Attachment is obsolete: true
Attachment #8530303 -
Flags: review?(past)
Attachment #8530306 -
Flags: review?(past)
Assignee | ||
Comment 5•9 years ago
|
||
New try build: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=56a09d790ebf
Comment 6•9 years ago
|
||
Comment on attachment 8530306 [details] [diff] [review] bug1106079-broken-styleeditor-xul v2.patch Review of attachment 8530306 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/styleeditor/StyleEditorUI.jsm @@ +127,5 @@ > + this._highlighter = > + yield hUtils.getHighlighterByType(SELECTOR_HIGHLIGHTER_TYPE); > + } catch (e) { > + // The selectorHighlighter can't always be instantiated, for example > + // it doesn't work with XUL windows. Perhaps add a reference to the bug that will make this try/catch unnecessary? ::: browser/devtools/styleeditor/test/browser_styleeditor_xul.js @@ +10,5 @@ > + > +let test = asyncTest(function*() { > + let tab = yield addTab(TEST_URL); > + let target = TargetFactory.forTab(tab); > + Nit: trailing whitespace @@ +14,5 @@ > + > + let toolbox = yield gDevTools.showToolbox(target, "styleeditor"); > + let panel = toolbox.getCurrentPanel(); > + yield panel.UI.once("editor-added"); > + Ditto.
Attachment #8530306 -
Flags: review?(past) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Thank you Panos for the ultra-fast review. Here's a new version with the comment added and the trailing whitespaces removed.
Attachment #8530306 -
Attachment is obsolete: true
Attachment #8530312 -
Flags: review+
Assignee | ||
Comment 8•9 years ago
|
||
Pushed to fx-team: https://hg.mozilla.org/integration/fx-team/rev/555d26af34d6
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/555d26af34d6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8530312 [details] [diff] [review] bug1106079-broken-styleeditor-xul v3.patch Approval Request Comment [Feature/regressing bug #]: This patch fixes a regression introduced by bug 985597. [User impact if declined]: If declined, the style-editor panel in the devtools won't work at all for XUL windows (so specifically, it won't work when using the browser-toolbox). [Describe test coverage new/current, TBPL]: Landed the fix in nightly last week, it's been there since, and a new mochitest was added. [Risks and why]: I can't see much risk in uplifting this patch. The actual fix is a try/catch around an existing piece of code in the style-editor. This try/catch makes sure we can start the editor, no matter which version of the debugger server we're connected to. [String/UUID change made/needed]: None.
Flags: needinfo?(pbrosset)
Attachment #8530312 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox36:
--- → affected
status-firefox37:
--- → fixed
Updated•9 years ago
|
Attachment #8530312 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
Flags: in-testsuite+
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•