1263439 - CSS rules inspector not showing rules when stylesheet has a data-uri sourcemap and link href is generated with `URL.createObjectURL`

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

Attachment: blob.html

When a stylesheet is inserted in a page using a blob and `URL.createObjectURL` , and has a data-uri sourcemap, the rule inspector don't show any rules.

STR:

1. Open attached file
2. Right-click -> inspect "Test" element ( which will open the inspector )

Expected result :
The Rules panel shows the rule for the h1 tag

Actual result :
The Rules is empty. Error is shown in Browser Toolbox : 
`
Full Message: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService.newURI]
Full Stack: JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: dirname :: line 1202
JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: StyleSheetActor<._setSourceMapRoot :: line 806
JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: StyleSheetActor<._fetchSourceMap/</map< :: line 779
JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 937
JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 816
`

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

An error was thrown : `
Full Message: Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIIOService.newURI]
Full Stack: JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: dirname :: line 1202
JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: StyleSheetActor<._setSourceMapRoot :: line 806
JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/stylesheets.js :: StyleSheetActor<._fetchSourceMap/</map< :: line 779
JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: Handler.prototype.process :: line 937
JS frame :: resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js :: this.PromiseWalker.walkerLoop :: line 816
`
I stripped out the "blob:" part of the safeUrl before calling the `dirname` function and it resolves the bug.
However, I don't know if it is a proper fix ( do we want the sourceMapRoot url without the "blob:" part ? ).
If it is, I don't know if we can move it to the safeHref getter instead.

Review commit: https://reviewboard.mozilla.org/r/45369/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45369/

Comment 2

[Loic]

Another regression of bug 1243736.

Comment 3

[Loic]

Bug 1243736 has introduced 3 regressions: bug 1249888 (fixed in 46+), bug 1255787 (only fixed in 48) and the current bug 1263439 (waiting for fix). Could we backport the last 2 patches in 47 at least?

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

https://reviewboard.mozilla.org/r/45369/#review42287

This seems to work fine. Thanks.
I'd like to see a test for it though. It should be an easy one. Create a new doc_blob_stylesheet.html file in devtools/client/inspector/rules/test and a new test browser_rules_blob_stylesheet.js that goes with it. The doc page should craft a blob URL like the attachment you added to the bug, and the test should just open it and assert that there are rules displayed in the sidebar, nothing more.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45369/diff/1-2/

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45369/diff/2-3/

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

https://reviewboard.mozilla.org/r/45369/#review42299

Looks good, thanks!

Just a minor comment about the test html file, which you can address without asking for a new review.

Also, the first line of the commit message should try and say what you changed, rather than explain the problem only. Maybe something like this:

Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

And then go into more details on the following lines (although I'm not sure the error message should be in there).

::: devtools/client/inspector/rules/test/doc_blob_stylesheet.html:1
(Diff revision 2)
> +<head>



::: devtools/client/inspector/rules/test/doc_blob_stylesheet.html:1
(Diff revision 2)
> +<head>

Please add the license header that we have in other html files and while you're at it, maybe make it a complete HTML document with doctype, html, and meta utf-8.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45369/diff/3-4/

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Thanks loic, patrick, nicolas, for digging into this. 
We could still take a patch in 47 but it is getting late for 46 uplift. 
If you disagree and think it is important (and safe for beta 11) please let me know.

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

TRY run is over : https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dd552b782de

Except from a known intermittent (https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dd552b782de&selectedJob=19367794), everything looks good

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d6a124bce670

Comment 12

[Patrick Brosset <:pbro>]

This was backed out from fx-team for eslint failures.
Here are the failures:


TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/browser_rules_blob_stylesheet.js:10:18 | Missing space after *. (generator-star-spacing)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:13:1 | Use the global form of 'use strict'. (strict)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:19:1 | Line 19 exceeds the maximum line length of 80. (max-len)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:20:45 | Strings must use doublequote. (quotes)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:23:11 | Multiple spaces found before '='. (no-multi-spaces)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:23:36 | Strings must use doublequote. (quotes)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:24:11 | Multiple spaces found before '='. (no-multi-spaces)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:24:36 | Strings must use doublequote. (quotes)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:25:11 | Multiple spaces found before '='. (no-multi-spaces)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:25:13 | Strings must use doublequote. (quotes)
TEST-UNEXPECTED-ERROR | devtools/client/inspector/rules/test/doc_blob_stylesheet.html:26:13 | Strings must use doublequote. (quotes)

Comment 13

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/ae00188ceb4d

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45369/diff/4-5/

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Nicolas Chevobbe from comment #14)
> Comment on attachment 8739773 [details]
> MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created
> with URL.createObjectURL; r=pbro
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/45369/diff/4-5/

I fixed the eslint errors and now both `./mach eslint devtools/client/inspector/rules/test/browser_rules_blob_stylesheet.js`and `/mach eslint devtools/client/inspector/rules/test/doc_blob_stylesheet.html` output `No errors encountered.`.

As discussed with pbro on IRC, I put this as checkin-needed again

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/a83cf78360d1

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a83cf78360d1

Comment 18

[edimoldovan]

Big thank you to everyone who worked on this. Highly appreciated.

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Patrick, Nicolas, this is marked as a P1 regression. Should we consider uplifting this fix to Beta47?

Comment 20

[Nicolas Chevobbe [:nchevobbe]]

Sorry I don't know what's the protocol to uplift a patch. Patrick would know better.

Comment 21

[Patrick Brosset <:pbro>]

(In reply to Ritu Kothari (:ritu) from comment #19)
> Hi Patrick, Nicolas, this is marked as a P1 regression. Should we consider
> uplifting this fix to Beta47?
Hard to tell. I have no idea how many developers can be affected by this. The fix is short and safe though, so I'll request uplift to beta.

Comment 22

[Patrick Brosset <:pbro>]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Approval Request Comment
[Feature/regressing bug #]: 1243736
[User impact if declined]: Low impact. This only impacts devtools users who open the inspector on a site that loads a stylesheet with a blob URL, and when this stylesheet defines a source map with a data-uri.
When that happens, the inspector's rule-view is empty (no css rules displayed).
[Describe test coverage new/current, TreeHerder]: A new automated test was added. And this fix has landed a while back and has not caused any regressions in nightly and aurora so far.
[Risks and why]: Minimal risk for the reason stated above. Should there be a problem anyway, then it would only affect the rule-view in the inspector of devtools.
[String/UUID change made/needed]: None

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Nicolas, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8739773 [details]
MozReview Request: Bug 1263439 - Remove "blob:" from stylesheet URLs created with URL.createObjectURL; r=pbro

Recent regression in devtools inspector, baked in Nightly for ~2 weeks, Beta47+

Comment 25

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Ritu Kothari (:ritu) from comment #23)
> Hello Nicolas, could you please verify this issue is fixed as expected on a
> latest Nightly build? Thanks!

Following steps from Comment 0 in Nightly 49.0a1 (2016-05-02), I do see the expected rules, so this issue is actually fixed.

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b81fd11b6631

Comment 27

[Tanvir Rahman]

I have reproduced this bug with nightly 48.0a1 (2016-04-10) on Windows 10, 64 bit!

The Bug's fix is now verified on Release 47.0 and Beta 48.0b6.

Firefox 47.0
Build ID 	20160604131506
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0

Beta 48.0b6
Build ID 	20160706215822
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

 [bugday-20160713]