" Error: Unknown sheet source" error in jsconsole when creating a new stylesheet in the StyleEditor

RESOLVED FIXED in Firefox 50

Status

()

Firefox
Developer Tools: Style Editor
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

49 Branch
Firefox 50
Points:
---

Firefox Tracking Flags

(firefox49 affected, firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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

2 years ago
Assignee: nobody → chevobbe.nicolas
(Assignee)

Comment 1

2 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

2 years ago
Created attachment 8760415 [details]
Bug 1276468 - Fix `sheetToUrl` function for inline style.

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 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

2 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

2 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

2 years ago
Attachment #8760415 - Flags: review+ → review?(jwalker)
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

2 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
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

2 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

2 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

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0aab68e30c0e
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
You need to log in before you can comment on or make changes to this bug.