Closed
Bug 1276468
Opened 9 years ago
Closed 9 years ago
" Error: Unknown sheet source" error in jsconsole when creating a new stylesheet in the StyleEditor
Categories
(DevTools :: Style Editor, defect, P2)
Tracking
(firefox49 affected, firefox50 fixed)
RESOLVED
FIXED
Firefox 50
People
(Reporter: nchevobbe, Assigned: nchevobbe)
Details
Attachments
(1 file)
There is no impact on the tool itself, everything works fine, but the jsconsole is cluttered with error messages.
STR:
1. Open `data:text/html,<p>hello` url
2. Open the devtools
3. Go to the style editor tab
4. Click on the "New" button in the left panel
Expected Result:
No error in the jsconsole
Actual Result:
There is an error in the jsconsole
Full Error
Error: Unknown sheet source
Stack trace:
exports.sheetToUrl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/csscoverage.js:733:9
StyleEditorUI.prototype._sourceLoaded/<.onShow</<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:614:22
TaskImpl.prototype._run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:310:39
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Editor.prototype.appendTo/onLoad@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/sourceeditor/editor.js:456:7
EventListener.handleEvent*Editor.prototype.appendTo@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/sourceeditor/editor.js:459:5
StyleSheetEditor.prototype.load@resource://devtools/client/styleeditor/StyleSheetEditor.jsm:446:12
StyleEditorUI.prototype._sourceLoaded/<.onShow</<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:601:19
TaskImpl.prototype._run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:310:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:272:3
createAsyncFunction/asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:246:14
this.Task.spawn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:160:12
StyleEditorUI.prototype._sourceLoaded/<.onShow<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:596:9
set activeSummary@resource://devtools/client/shared/SplitView.jsm:147:7
StyleEditorUI.prototype._selectEditor/summaryPromise<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:702:7
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1281:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1223:7
generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1382:14
StyleSheetEditor.prototype._getSourceTextAndPrettify@resource://devtools/client/styleeditor/StyleSheetEditor.jsm:270:12
StyleSheetEditor.prototype.fetchSource@resource://devtools/client/styleeditor/StyleSheetEditor.jsm:290:12
StyleEditorUI.prototype._addStyleSheetEditor<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:348:11
TaskImpl.prototype._run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:310:39
TaskImpl@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:272:3
createAsyncFunction/asyncFunction@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:246:14
StyleEditorUI.prototype._onStyleSheetCreated@resource://devtools/client/styleeditor/StyleEditorUI.jsm:398:5
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
Front<.onPacket/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1281:9
DevTools RDP*Front<.request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1223:7
generateRequestMethods/</frontProto[name]@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1382:14
StyleEditorUI.prototype.createUI/<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:161:7
EventListener.handleEvent*wire/<@resource://devtools/client/styleeditor/StyleEditorUtil.jsm:161:7
forEach@resource://devtools/client/styleeditor/StyleEditorUtil.jsm:106:7
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → chevobbe.nicolas
Assignee | ||
Comment 1•9 years ago
|
||
This is not limited to added new rule, but seems to appear also on page with inline <style> tag.
Another STR :
1. Open : `data:text/html,<style>body{color: blue;}</style><p>hello`
2. Open the devtools and go to the style editor
3. Normally, the stylesheet is automatically selected, if not, select it
You should see the error mentioned in Comment 0
Assignee | ||
Comment 2•9 years ago
|
||
The function was trying to access StyleSheetActor's ownerNode property, which is undefined.
Switch to nodeHref and styleSheetIndex properties to build the so-called URL.
The built string is only used later in the same file for comparison with another string,
built with the same function, so the syntax of the string does not really matter as long
it is unique.
Review commit: https://reviewboard.mozilla.org/r/57972/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57972/
Attachment #8760415 -
Flags: review?(jwalker)
Comment 3•9 years ago
|
||
Comment on attachment 8760415 [details]
Bug 1276468 - Fix `sheetToUrl` function for inline style.
https://reviewboard.mozilla.org/r/57972/#review55004
Looks good to me, thanks!
Attachment #8760415 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8760415 [details]
Bug 1276468 - Fix `sheetToUrl` function for inline style.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57972/diff/1-2/
Attachment #8760415 -
Attachment description: Bug 1276468 - Fix function for inline style. → Bug 1276468 - Fix `sheetToUrl` function for inline style.
Assignee | ||
Comment 5•9 years ago
|
||
TRY run raised some failures https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9fe131ebd8d , mostly in the gcli tool.
It turns out that the function can be called either with a plain StyleSheet or with a StyleSheetActor.
For now, I just added a simple case for when we have an actor and retrieve similar properties to when we have a StyleSheet.
Although, I don't know if it's a good behavior to have a function that can be called with 2 different types of parameter, but this is a relative concise function and the types are related.
We could avoir this by always passing in a StyleSheet parameter, but this would either (1) require extra work, i.e. go through the CSSRule to have access to the parentStyleSheet property (which is what's done in the function call done by the gcli), or (2) adding a `stylesheet`property to the StyleSheetActor, which I think is not worthy just to get around this.
Please, feel free to tell me if you think I'm wrong and we should handle this more gracefully.
Assignee | ||
Updated•9 years ago
|
Attachment #8760415 -
Flags: review+ → review?(jwalker)
Comment 6•9 years ago
|
||
Comment on attachment 8760415 [details]
Bug 1276468 - Fix `sheetToUrl` function for inline style.
https://reviewboard.mozilla.org/r/57972/#review55148
My take on the question of parameter overloading is that it's OK if the 2 different set of possible arguments are disjoint sets with no possibility of overlap.
So it's OK to have a parameter which is either a Person object or their social security ID (for example).
But java.util.List.remove(x) when x can be either an index or the instance to be removed, is dangerous because list.remove(3) is confusing.
Attachment #8760415 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 7•9 years ago
|
||
TRY run is over https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eca1c1f9709&selectedJob=22122677
There is 2 dt* failures, but they seem unrelated and they've been successfully retriggered.
Keywords: checkin-needed
Comment 8•9 years ago
|
||
has problems to apply:
applying 95d72f6eed1f
patching file devtools/server/actors/csscoverage.js
Hunk #1 FAILED at 709
1 out of 1 hunks FAILED -- saving rejects to file devtools/server/actors/csscoverage.js.rej
patch failed to apply
abort: fix up the working directory and run hg transplant --continue
could you take a look, thanks!
Flags: needinfo?(chevobbe.nicolas)
Keywords: checkin-needed
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8760415 [details]
Bug 1276468 - Fix `sheetToUrl` function for inline style.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57972/diff/2-3/
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #8)
> has problems to apply:
>
> applying 95d72f6eed1f
> patching file devtools/server/actors/csscoverage.js
> Hunk #1 FAILED at 709
> 1 out of 1 hunks FAILED -- saving rejects to file
> devtools/server/actors/csscoverage.js.rej
> patch failed to apply
> abort: fix up the working directory and run hg transplant --continue
>
> could you take a look, thanks!
I rebased (see Comment 9) and pushed to patch to review again. Can you tell me if it's okay for you ?
Also, unrelated to you, but the STR in Comment 0 now shows a new error, introduced by https://hg.mozilla.org/mozilla-central/diff/a325b8ebc115/devtools/client/styleeditor/StyleEditorUI.jsm (the function is called on the front but only exists in the actor). I'll file a new bug for this.
Flags: needinfo?(chevobbe.nicolas)
Comment 11•9 years ago
|
||
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/0aab68e30c0e
Fix `sheetToUrl` function for inline style. r=jwalker
Comment 12•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•