1322013 - CSS rules panel for elements with styles affected by CSS injection via content_scripts (WebExtension)

Status: VERIFIED FIXED

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

Description

[Pedro Cunha]

Attachment: extension+screenshots.zip

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161130094234

Steps to reproduce:

1. Start firefox (if possible with new profile)
2. Goto about:debugging and load the temporary extension provided in the attached .zip file.
3. Load the http://example.com webpage, and inspect the <h1> element there.


Actual results:

The <h1> element turns red (that's what the extension does), and the Rules panel is empty when we try to inspect the element.


Expected results:

The <h1> element should turn red and the  Rules panel should also contain CSS rules affecting the element being inspected.

Firefox outputs the following in the terminal:
console.error: 
  Protocol error (unknownError): Failed to open input source 'moz-extension://dd9af389-e3bd-4efd-90e7-93167f087774/style.css'

I have also provided screenshots of the behaviour seen.

Comment 1

[Julian Descottes [:jdescottes]]

P1, we should check if this is reproducible consistently.

Comment 2

[Julian Descottes [:jdescottes]]

Bug confirmed, issue with the principal used to download a moz-extension:// stylesheet.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1322013 - Use system principal to fetch moz-extension stylesheets;

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

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8821859 [details]
Bug 1322013 - Use system principal to fetch moz-extension stylesheets;

https://reviewboard.mozilla.org/r/100974/#review101542

::: devtools/server/actors/stylesheets.js:450
(Diff revision 1)
> +      this.text = content;
> +      return content;
> +    });
> +  },
> +
> +  fetchStylesheet: Task.async(function* (href) {

Add a JSDoc for this.

::: devtools/server/actors/stylesheets.js:465
(Diff revision 1)
>      // for preventing the assertion of the userContextId mismatching.
>      // The default internal stylesheets load from the 'resource:' URL.
>      // Bug 1287607, 1291321 - 'chrome' and 'file' protocols should also be handled in the
>      // same way.
> -    if (!/^(chrome|file|resource):\/\//.test(this.href)) {
> +    let excludedProtocolsRe = /^(chrome|file|resource|moz-extension):\/\//;
> +    if (!excludedProtocolsRe.test(this.href)) {

Can you add a comment that states that we use the content principal when the protocols are excluded in this condition

::: devtools/server/actors/stylesheets.js:481
(Diff revision 1)
> +      console.error(`stylesheets actor: fetch failed for ${this.href},` +
> +        ` using system principal instead.`);
> +      options.window = undefined;
> +      options.principal = undefined;
> +      result = yield fetch(this.href, options);
> +    }

Add a new line after the try block

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8821859 [details]
Bug 1322013 - Use system principal to fetch moz-extension stylesheets;

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

Comment 6

[Julian Descottes [:jdescottes]]

Thanks for the review. Updated the patch and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=027910ca5e5b195462e1e69b4478f38cb3890c44

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8821859 [details]
Bug 1322013 - Use system principal to fetch moz-extension stylesheets;

https://reviewboard.mozilla.org/r/100974/#review101624

::: devtools/server/actors/stylesheets.js:454
(Diff revision 2)
> +
> +  /**
> +   * Fetch a stylesheet at the provided URL. Returns a promise that will resolve the
> +   * result of the fetch command.
> +   *
> +   * @param {String} href

Adding one more space after @param will make the {String} align with the {Promise}

Comment 8

[Julian Descottes [:jdescottes]]

(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #7)
> Comment on attachment 8821859 [details]
> Bug 1322013 - Use system principal to fetch moz-extension stylesheets;
> 
> https://reviewboard.mozilla.org/r/100974/#review101624
> 
> ::: devtools/server/actors/stylesheets.js:454
> (Diff revision 2)
> > +
> > +  /**
> > +   * Fetch a stylesheet at the provided URL. Returns a promise that will resolve the
> > +   * result of the fetch command.
> > +   *
> > +   * @param {String} href
> 
> Adding one more space after @param will make the {String} align with the
> {Promise}

I don't mind either way, but we should either document this or enforce it by linting.

Comment 9

[Julian Descottes [:jdescottes]]

Comment on attachment 8821859 [details]
Bug 1322013 - Use system principal to fetch moz-extension stylesheets;

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4cbc57e29e25
Use system principal to fetch moz-extension stylesheets;r=gl

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4cbc57e29e25

Comment 12

[Md.Tarikul Islam Oashi]

I have to reproduced this bug with Nightly 53.0a1 on Windows 7 64 bit;

The Bug's fix is verified on Latest Nightly 

Build ID 	20170104030214
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170104]

Comment 13

[Mihai Boldan, Desktop QA [:mboldan]]

Thanks Md.Tarikul Islam Oashi for helping us with this bugs.
I've also verified this issue on Latest Nightly, under Mac OS X 10.12.1 and under Ubuntu 14.04x64.
I'm marking this issue Verified Fixed.