Closed
Bug 1068555
Opened 10 years ago
Closed 9 years ago
"adguard-adblocker" add-on does not work with e10s
Categories
(Firefox :: Extension Compatibility, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: artems777, Unassigned)
References
()
Details
(Keywords: addon-compat)
Summary: adguard-adblocker add-on does not work with e10s → "adguard-adblocker" add-on does not work with e10s
Keywords: addon-compat
Blocks: e10s-addons
Updated•10 years ago
|
tracking-e10s:
--- → +
Comment 1•10 years ago
|
||
The developer has been notified through AMO.
Comment 2•10 years ago
|
||
Hello!
We've tested Adguard with e10s and the bug is rather weird.
Looks like our implementation of nsIContentPolicy does not work for most of requests.
Firefox does not call shouldLoad method for HTTP requests.
But it does call it for requests like "about:newtab" or "chrome://global/skin/icons/close.png".
Comment 3•10 years ago
|
||
Looks like we should use nsICategoryManager from Compatibility shims to fix this on our side.
Comment 4•10 years ago
|
||
We've looked into it and it looks like there is a bug with bootstrapped add-ons.
Here are two test add-ons:
https://www.dropbox.com/sh/e8g3kurnqqegqh9/AAAX8eStAZ-anLluJ7cj0RNta?dl=0
They are very simple. Both implement nsIContentPolicy and write intercepted requests to log.
TestContentPolicySdk: sdk add-on built with cfx
TestContentPolicyBootstrap: bootstrapped add-on
If browser.tabs.remote.autostart is disabled both add-ons works ok and nsIContentPolicy works for all requests.
If browser.tabs.remote.autostart is enabled:
1. TestContentPolicySdk is ok
2. TestContentPolicyBootstrap is not. shouldLoad does not called for HTTP requests.
Comment 5•10 years ago
|
||
Ally: does Andrey's nsIContentPolicy test case look like a Jetpack bug?
Flags: needinfo?(ally)
Updated•10 years ago
|
Flags: needinfo?(ally)
Comment 7•10 years ago
|
||
(In reply to Chris Peterson (:cpeterson) from comment #5)
> Ally: does Andrey's nsIContentPolicy test case look like a Jetpack bug?
Andrey says it works in the SDK addon, but not in a plain bootstrap one:
(In reply to Andrey Meshkov from comment #4)
> If browser.tabs.remote.autostart is enabled:
> 1. TestContentPolicySdk is ok
> 2. TestContentPolicyBootstrap is not. shouldLoad does not called for HTTP
> requests.
which is weird. SDK doesn't have to do anything with nsIContentPolicy, as it's not an API we provide, so using it from an SDK addon should behave the same as using it from any other bootstrap addon (currently, SDK addons are nothing more than bootstrap addons with some modules.
the only thing that comes to mind is that the shims are not loaded, or not _yet_ loaded by the time the bootstrap code runs?
Andrey, can you please re-test with this in mind? maybe delay the running of your main addon code to see if we have a bug in when shims are loaded?
Flags: needinfo?(tomica+amo) → needinfo?(am)
Comment 8•10 years ago
|
||
Hi Tomislav!
1. Sorry, I've confused add-on types in my previous message.
TestContentPolicyBootstrap is ok, but TestContentPolicySdk is not.
2. We've done some debugging and found out the place where content policy is added.
RemoteAddonsParent.jsm:
------------------------------
CategoryManagerInterposition.methods.addCategoryEntry =
function(addon, target, category, entry, value, persist, replace) {
if (category == "content-policy") {
ContentPolicyParent.addContentPolicy(entry);
}
target.addCategoryEntry(category, entry, value, persist, replace);
};
In case of bootstrapped add-on this method is called and everything works ok.
In case of SDK add-on this method is not called at all.
We've tried to add 5 sec timeout but nothing has changed.
Flags: needinfo?(am)
Comment 9•10 years ago
|
||
Andrey, are you still blocked by the issue on comment #8?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(am)
Comment 10•10 years ago
|
||
Hi Jorge!
We've just tested in the latest nightly and it seems like this bug is now fixed.
Flags: needinfo?(am)
Comment 11•10 years ago
|
||
That's great news. Let us know when the new version has been submitted to AMO for review and I'll try to give it priority. We'll close this bug when the fixed version is publicly available.
Comment 12•10 years ago
|
||
You are in time:) Just uploaded it, version 2.0.4
Comment 13•9 years ago
|
||
The latest version seems to work fine, poking Jorge.
Flags: needinfo?(jorge)
Comment 14•9 years ago
|
||
Yep, we've fixed everything in 2.0.13.
Sorry, I should have left a comment here.
Thank you Jerod!
Comment 15•9 years ago
|
||
The latest version is now up on AMO.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(jorge)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•