Closed Bug 1380156 Opened 7 years ago Closed 7 years ago

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

Categories

(WebExtensions :: Request Handling, defect, P1)

56 Branch
defect

Tracking

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

VERIFIED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- verified

People

(Reporter: mrdokenny, Assigned: kmag)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image 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.
Blocks: webext-oop
Keywords: regression
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
(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.
(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.
Workaround is to set extensions.webextensions.protocol.remote = false and restart (extensions.webextensions.remote = true can be left alone).
Is this related to the recent changes you made Haik?
Flags: needinfo?(haftandilian)
Priority: -- → P1
Component: WebExtensions: Untriaged → WebExtensions: Developer Tools
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.
Assignee: nobody → kmaglione+bmo
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.
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
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 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+
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
https://hg.mozilla.org/mozilla-central/rev/ee7761772725
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Attached image 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
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.