Inject our stylesheet on per-site basis rather than globally

[trev]

Background

Currently we register a large global stylesheet in Firefox, it is being applied to every single webpage. According to ​https://bugzilla.mozilla.org/show_bug.cgi?id=988266 there is no memory use advantage however, essentially a copy of this stylesheet is being injected into each document.

What to change

We can stick to a single stylesheet file for now and merely decide individually on whether we want to inject it into a webpage. We can listen to some notification like content-document-global-created and use ​nsIDOMWindowUtils.loadSheet() to inject the stylesheet. This will solve several issues:

• No stylesheet is injected into whitelisted pages and pages where element hiding is disabled - meaning also no side-effects from element hiding that were unavoidable so far.
• The ugly protocol whitelist for generic rules can go, we simply won't inject the stylesheet into chrome:// documents and such.
• More consistent behavior - changes to element hiding rules apply only after page reload, same as with other filters.
• No hang when Adblock Plus is enabled or disabled globally coming from the stylesheet being injected into all open tabs simultaneously.
• We can get rid of the current ugly approach to implement exceptions via -moz-binding once ​bug 349813 is fixed.

[tschuster]

I will try to prototype this.

[tschuster]

• The ugly protocol whitelist for generic rules can go, we simply won't inject the stylesheet into chrome:// documents and such.

I think if we use "content-document-global-created" we are guaranteed not to inject into chrome :)

• More consistent behavior - changes to element hiding rules apply only after page reload, same as with other filters.

Perfect, if we had to apply changes immediately this would turn out be rather difficult.

[trev]

Replying to tschuster:

I think if we use "content-document-global-created" we are guaranteed not to inject into chrome :)

content-document-global-created is about the distinction between chrome and content meaning which side of the security boundary you are on - it isn't about privileges. Privileged documents can be loaded into the content area (e.g. Add-ons Manager page).

[tschuster]

I just uploaded a patch for this. It seems to work correctly for me, but probably not a final version.

[philll]

By tschuster:
So, just to update this bug. I recently uploaded a version of this patch that is almost finished. Sadly this regresses memory usage, because now we have more style-sheet memory per window. I talked to bz about this and he suggested that we add an API to Gecko, which would allow us to cache one global stylesheet and just load a reference to it in every window. I want to spend some time to figure out if this is something I can do easily. That new API should put as at slightly better memory usage. And as a next step we can probably split the css in two, one global css that gets loaded in every window and one css with just the domain specific rules, if they exist.

[arthur]

Would this fix the follwing issue?

1. Update EasyList
2. Add these 3 filters: ##.banner300x250, ##.banner300x100 & ###sponsored-ad
3. Go to ​this page and notice the anti adblock message
4. Add these 3 whitelists: rai.tv#@#.banner300x250, rai.tv#@#.banner300x100 & rai.tv#@##sponsored-ad
5. Reload the page and notice the anti adblock message still appears
6. Remove the filters and whitelists and notice the message doesn't show up, the video appears instead

This doesn't happen in Chrome. It works when you add the whitelists.

[trev]

Yes, it would fix that issue.

[arthur]

Is it possible to estimate when this will be ready? In order to counteract a certain video fallback ad mechanism that is being used on some big German sites I have to disable generic element hiding filters because an element hiding whitelist doesn't work as it should in Firefox.

[tschuster]

It would be interested in understanding how my patch helps there. Anyway I just uploaded a new version. I am not sure when it's read, this depends on what Wladimir or anybody else finds during review and how complicated this is to address.

[archon810]

Looks like ​https://adblockplus.org/forum/viewtopic.php?f=2&t=25867&p=109480#p109480 is affected by this bug. Is there any idea if it's going to be fixed any time soon please?

Thank you.

[archon810]

Hi,

Any plans to fix this issue? It's still affecting the plugin and our sites.

Thank you.

[Compuitguy]

How can we vote/cc for this ?

[mapx]

it's under review (last activity 8 months ago ...) ​https://codereview.adblockplus.org/6201308310667264/

[Compuitguy]

Yes, saw that

So there is no possibility to vote/cc for issues ?

[Compuitguy]

Thanks mapx

BTW,

There is progress on this bugzilla issue which is related

​77999 – share rule processors for XUL, scoped, UA, and user stylesheets

[Compuitguy]

​77999 – share rule processors for XUL, scoped, UA, and user stylesheets

Status: RESOLVED FIXED

Target Milestone: mozilla41

From the bug:

ABP is enabled, using its default blocklist, we save a bit over 2 MB per document. (This is for every displayed document, including iframes, except one, since we are now sharing this data between documents.)

Without ABP, we just have the standard UA style sheets that Firefox adds to every document, and in that case we save about 120 KB per document.

We also save the time required to build up these tables, but I haven't measured that.

[trev]

@tschuster: Does fixing this still make sense given that bug 77999 has been resolved?

[tschuster]

Not really. I see no way this is going to reduce memory below that what we have after fixing bug 77999.

[arthur]

Still there is the issue where Google ads embedded with the responsive Adsense code will not show up if you disable Adblock Plus for a specific domain (e. g. ​http://www.thewildernist.org/2015/07/history-bison-southeastern-north-america/, tested with Nightly 42.0a1 (20150723030207) and Adblock Plus 2.6.9.3966 with EasyList only). Whitelisting those in Acceptable Ads in Firefox doesn't work then.

Should I create a new issue for that?

[mapx]

still:

test using 42.0a2 (2015-08-15)
tested also in 41.0b1

go to ​http://apkone.net/
now you should add:
@@.png#$domain=apkone.net

keeping easylist enabled does not work
disabling easylist and adding easylist without hiding filters ==> will work

[tschuster]

"keeping easylist enabled does not work" What do you mean exactly?
When visting this site I get some kind of warning about using an Adblocker, which makes this site unusable. Is it that?

[mapx]

if you use easylist and add the new filter
@@.png#$domain=apkone.net

the site will work fine in ABP for chrome but not in ABP for firefox and this due to the old bug with stylesheets. It will work only if you use easylist without hiding filters. And this even if in easylist we have the whitelisting filter

@@||apkone.net^$elemhide

see also https://issues.adblockplus.org/ticket/521#comment:14

and the comments of Wladimir (not only here but also on blog) which indicates this issue as a consequence of 521

[mapx]

I guess https://issues.adblockplus.org/ticket/521#comment:33
it's the same issue

[tschuster]

Mhm I guess per-site filtering would probably fix this, but this doesn't mean we can't fix it in another way. I will look into it.

[mapx]

Replying to tschuster:

Mhm I guess per-site filtering would probably fix this, but this doesn't mean we can't fix it in another way. I will look into it.

still nothing ? very old bug.

another test case:
​http://www.cinema2satu.net/

the filters already in easylist are working fine in ABP for chrome
in firefox still present the anti adblock message
tested also adding

@@||cinema2satu.net^$generichide

does not work.

It's working only switching to easylist without element hiding.

[trev]

We still want to tackle this but I think P3 is more realistic.

[trev]

Tom, hope you don't mind me taking this. I've done some preparation work for this and #524 in #4139, it should be fairly simple now.

[abpbot]

A commit referencing this issue has landed:
​Issue 521 - Inject our stylesheet on per-site basis rather than globally

[mapx]

error in console on START (effect: the hiding filters are not working)

NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDOMWindowUtils.addSheet]

row code error (elemhide.js:346)

        utils.addSheet(this.sheet, Ci.nsIStyleSheetService.USER_SHEET);

[trev]

@mapx: I created #4227 for this regression.