Loading temporary an unpacked extension breaks extension page's CSS in OOP Extensions

VERIFIED FIXED in Firefox 56

Status

P1
normal
VERIFIED FIXED
a year ago
2 months ago

People

(Reporter: mrdokenny, Assigned: kmag)

Tracking

(Blocks: 1 bug, {regression})

56 Branch
mozilla56
regression
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 verified)

Details

MozReview Requests

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

Attachments

(3 attachments)

(Reporter)

Description

a year ago
Created attachment 8885463 [details]
Broken CSS.png

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

Steps to reproduce:

1. Download an extension xpi

Ex. 
https://addons.mozilla.org/en-US/firefox/addon/smart-https-revived/

2. Unzip the xpi (this is important)

3. Open about:debugging and load the extension's manifest.json in "Load a Temporary Add-on"


Actual results:

- The browser popup and settings menu is missing the css


Expected results:

- The css should have appeared. Also the css appears if you load the xpi (packed or install it from AMO. Only when the extension is unpacked/unzipped the css doesn't appear. This also used to work until after OOP extensions was enabled.
(Reporter)

Updated

a year ago
Blocks: 1190679
Keywords: regression
(Assignee)

Updated

a year ago
Blocks: 1334550
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
Depends on: 1380186
I've a suspicion this may be fixed by the parent/child channel work I'm doing in bug 1380186
Duplicate of this bug: 1380276
(In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> I've a suspicion this may be fixed by the parent/child channel work I'm
> doing in bug 1380186

I've a suspicion my suspicion is unfounded.
(Assignee)

Comment 4

a year ago
(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> > I've a suspicion this may be fixed by the parent/child channel work I'm
> > doing in bug 1380186
> 
> I've a suspicion my suspicion is unfounded.

It's probably unfounded. This is almost certainly caused by moz-extension protocol remoting.
(Assignee)

Updated

a year ago
Duplicate of this bug: 1381346

Comment 6

a year ago
Workaround is to set extensions.webextensions.protocol.remote = false and restart (extensions.webextensions.remote = true can be left alone).

Comment 7

a year ago
Is this related to the recent changes you made Haik?
Flags: needinfo?(haftandilian)
Priority: -- → P1

Updated

a year ago
Component: WebExtensions: Untriaged → WebExtensions: Developer Tools
(Assignee)

Updated

a year ago
Status: UNCONFIRMED → NEW
Component: WebExtensions: Developer Tools → WebExtensions: Request Handling
Ever confirmed: true
(In reply to Andy McKay [:andym] from comment #7)
> Is this related to the recent changes you made Haik?

I'm still testing, but I don't think so. For me, on my local Mac Nightly build, it is reproducible with/without OOP extensions and with/without moz-extension protocol remoting. Essentially, I can't get it to work with any combination of prefs.

In comment 6 Kestrel reported setting extensions.webextensions.protocol.remote=false was a workaround, but that isn't the case for me.

Note: On Mac, if OOP extensions are enabled and protocol remoting is disabled, the content sandbox read-access restrictions must also be disabled by changing security.sandbox.content.level. For Mac Nightly, set it to 2. For Mac Beta/Release, set it to 1. (We don't have a separate sandbox level setting for the extension process.) Without changing this, the addons page will show an access denied error.
Flags: needinfo?(haftandilian)
(In reply to Haik Aftandilian [:haik] from comment #8)
> Note: On Mac, if OOP extensions are enabled and protocol remoting is
> disabled, the content sandbox read-access restrictions must also be disabled
> by changing security.sandbox.content.level. For Mac Nightly, set it to 2.
> For Mac Beta/Release, set it to 1. (We don't have a separate sandbox level
> setting for the extension process.) Without changing this, the addons page
> will show an access denied error.

This only applies to Nightly at the moment because Mac read-access restrictions are only in Nightly at this time.

Updated

a year ago
Duplicate of this bug: 1379943

Updated

a year ago
Assignee: nobody → kmaglione+bmo
(Assignee)

Updated

a year ago
No longer depends on: 1380186
> In comment 6 Kestrel reported setting extensions.webextensions.protocol.remote=false was a workaround, but that isn't the case for me.

In Nightly Ubuntu Linux I see the issue despite the pref not helping as it's off for me already.
(Assignee)

Updated

a year ago
Duplicate of this bug: 1383994

Comment 13

a year ago
I double checked. In Nightly, Windows 10, clean profile, extensions.webextensions.protocol.remote=false definitely allows temporarily loaded extensions to load CSS via content_scripts manifest as long as they are not located in a user folder like Documents or Downloads. Extensions located in user folders will fail to load CSS regardless of the remote prefs and is regressed by Bug 1334550.
(In reply to Kestrel from comment #13)
> I double checked. In Nightly, Windows 10, clean profile,
> extensions.webextensions.protocol.remote=false definitely allows temporarily
> loaded extensions to load CSS via content_scripts manifest as long as they
> are not located in a user folder like Documents or Downloads. Extensions
> located in user folders will fail to load CSS regardless of the remote prefs
> and is regressed by Bug 1334550.

I am able to reproduce a CSS loading problem on Windows only and I think it is caused by bug 1334550. I tested the beastify[1] extension on Windows and found the popup menu CSS fails to load due to a remoting issue introduced by 1334550. Setting extensions.webextensions.protocol.remote=false and security.sandbox.content.level=2 let it work again. At present, I don't know why this only occurs on Windows. I'll work on this and update the bug with what I find.

I don't know yet if this is the same problem that is reported in the description which is an issue with extension page CSS.

[1] https://github.com/mdn/webextensions-examples/tree/master/beastify
Comment hidden (mozreview-request)
The problem here is caused by a bug in the extension remoting. In ExtensionProtocolHandler::NewStream(), we handle a URI sent from the child and create a channel for it. We make sure the channel is an nsIFileChannel because we only expect to be be handling remoted file:// URI child requests in NewStream(). That's a bug because when the file is a css file, the parent's call to ExtensionProtocolHandler::SubsituteChannel() replaces the channel with a SimpleChannel in order to do an async css conversion. We get into SubstituteChannel() because NewStream() calls NewChannel() with the child URI which goes back into the ExtensionProtocolHandler.

On Mac, where the extension process is not yet enabled by default, the value of extensions.webextensions.protocol.remote doesn't affect this because addon page css isn't remoted, it is loaded by the parent. As to why I thought the problem was occurring regardless of the extensions.webextensions.remote setting. I think I was confused about what the addon page was supposed to look like with working css. Apologies for that. Thanks to Kestrel for double checking.

The posted fix changes NewStream() to use the resolved URI instead of the child moz-extension URI to get a file channel. Using the child URI routes the request through ExtensionProtocolHandler::NewChannel2(), but that isn't necessary because we know the child URI should resolve to a file:/// and isn't one of the special cases handled in ExtensionProtocolHandler::ResolveSpecialCases(). We also shouldn't need the css conversion to be done in the parent. The change results in the css conversion being done in the child for unpacked extensions. It's already being done in the child for packed extensions.

Comment 17

a year ago
mozreview-review
Comment on attachment 8890207 [details]
Bug 1380156 - Loading temporary unpacked extension breaks extension page's CSS in OOP Extensions.

https://reviewboard.mozilla.org/r/161270/#review166722
Attachment #8890207 - Flags: review?(jmathies) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 20

a year ago
Pushed by haftandilian@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ee7761772725
Loading temporary unpacked extension breaks extension page's CSS in OOP Extensions. r=jimm

Comment 21

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ee7761772725
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Updated

a year ago
Duplicate of this bug: 1382180
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
status-firefox-esr52: --- → unaffected
Duplicate of this bug: 1385188
Created attachment 8893349 [details]
2017-08-03_1422.png

I was able to reproduce the initial issue on Firefox 56.0a1 (2017-07-09) under Windows 10 64-bit.
Verified as fixed on Firefox 56.0a1 (2017-08-01) and Firefox 57.0a1 (2017-08-02) under Windows 10 64-bit and Ubuntu 16.04 32-bit. See attached screenshot.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified

Updated

2 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.