Implement inline styles for hiding filters

[Lain_13]

Background

Hi, I'm maintainer of RU AdList - supplementary to EasyList list of filters for Russian and Ukrainian sites.

I periodically encounter situation when it is not possible to block ads in a convenient manner or not possible at all. Usually on sites with branding. For example look at the nnm.me.
They set image at the background through in-line stylesheet. It's possible to block it but it is not possible to remove a link from the body which is implemented as on-click event. Also there is no way to remove empty space above the content where branding were displayed since there are nothing to hide to get rid of it (it's implemented as padding of body element).

To resolve this I end up creating this style in Stylish:
@-moz-document domain("nnm.me") {

body {background:none!important;cursor:unset!important;}
#cntnt {margin-top:0px!important;left:0px!important;margin-left:0px!important;width:100%!important}}

And that is not a single example. There are many other. Part of them I covered with my supplementary list here: ​https://userstyles.org/styles/101141/ru-adlist-css-fixes
Not all filters there are imposible to implement without adblock but most of them are.

The problem is that ABP is only capable to hide elements. It's not capable to apply any other styles to a page.

What to change

I propose to add inline stylesheets as extension to normal hiding filters.

Example:
nnm.me##body {background:none!important;cursor:unset!important}
nnm.me###cntnt {margin-top:0px!important;left:0px!important;margin-left:0px!important;width:100%!important}

Old versions of ABP will treat such rules as blocking rules which doesn't block anything. So, they shouldn't create backward compatibility problems.

It's especially important because Adguard browser extensions does have such functionality.

More details are here: ​https://adblockplus.org/forum/viewtopic.php?f=1&t=11161&start=0

[mapx]

interesting this part:

"AdGuard browser extension does support such filters. They are using different syntax yet they do support them. Actually they even support JS injection!"

​https://adblockplus.org/forum/viewtopic.php?p=102421#p102421

[Lain_13]

mapx: It's done in a bit odd way since they add their API into each page (defined in the same JS injections rules as global injections) and that doesn't looks right to me but they really do support such feature. Look for "Javascript injections" section in their filters list: ​http://adguard.com/en/filter-rules.html?id=2

In comparison with possibility of tracking user with images or collection data on their display resolution this is significantly bigger. Yet they support it and seems like AdGuard users believe them to do no harm.

BTW, as I understoo they already implemented analogue of ticket #495.

[mapx]

about blocking inline script, another ticket
https://issues.adblockplus.org/ticket/748

[trev]

The main issue about JS injection or injection of random CSS styles isn't even the implementation but rather not compromising security of the user. The filter download server cannot be considered trusted, meaning that a compromised server (or filter author) shouldn't cause catastrophic issues - like intercepting user's login credentials on random servers. Generic CSS rules can be misused for tracking or to display additional content on a web page, in older Firefox versions they could also be used to run JavaScript code (not sure whether this issue is considered completely gone now). In other words, generic CSS code or generic JavaScript is not an option - merely some subset that can be considered safe and yet powerful enough to be useful.

[Lain_13]

trev: I think I've covered that part on the forum. Hovewer I've proposed to use blacklist for some specific things like ':before', ':after' and background with path to image which starting not from '/' or '.'. And probably ':visited' too. Also I think that such blacklist should be applied only to filters in the filters list. I think users should have ability to create any styles they want in their own filters.

p.s. I do agree that JS injection should be limited to a small subsed of functions rather being able to use anything but that is completely out of the scope of this feature request.

[trev]

A blacklist approach isn't going to work: CSS is a very flexible language and enforcing that certain constructs aren't being used is extremely complicated. Not to mention that there will always be language features that nobody thought about or simply feature introduced to CSS after we implement this functionality. A whitelist approach is the only one that might work.

[Lain_13]

Ok, but whitelist should be configurable from the about:config and/or in some hidden interface for debugging purposes. We can start from some limited set of properties and extend it with time.

I'll start with a list of properties used in my stylesheet + some additions:
top, bottom, left, right: any
width, max-width, min-width: any
height, max-height, min-height: any

margin(-leftrighttopbottom): any padding(-leftrighttopbottom): any

border, border-(anything but image and image-source): any
border-image, border-image-source: "none", "initial", "inherit"

background-(anything but image): any
background-image: "none", "initial", "inherit"
background: options must not contain characters "()%:/" (to prevent usage or urls and data-urls)
color: any
cursor: any (most usefult with "unset")
display: any
float: any
font, font-*: any
opacity: any

overflow(-xy): any (most useful to enable back scrolling disabled by modal windows)

position: any
resize: any (not useful in the filter lists but could be a nice thing to have for users)
transform(-*): any (most useful to set back to "none")
visibility: any
z-order: any

Is it possible to add keyword "!important" after each property automatically?
Is it possible to generate vendor-specific variations of properties?
It would be really annoying to add them manually.

Also it could be a good idea to allow first-party images in the background and *-image* properties. I've used such image once in my stylesheet to restore original background of the site. It could be complicated, though, and not important for initial release of the inline stylesheets feature. Data-urls should be forbidden to avoid vulnerabilities in the image decoders.

[mapx]

​https://github.com/gorhill/uBlock/wiki/Inline-script-tag-filtering

[trev]

That's not really related to the topic of this request (not is it really a useful feature from what I can tell).

[mario86]

I have created a new ticket, since this one is not related:
https://issues.adblockplus.org/ticket/3207#ticket

[Lain_13]

BTW, I still maintaining RU AdList CSS Fixes list and it only grew in popularity since last time. Right now it have ~18 000 users. It's still small in comparison with RU AdList users, but it's not installed by default, yet more than 18 thousands users went out of their way to install Stylish and CSS Fixes. Also, I have JS Fixes when CSS is not enough.

[Lain_13]

BTW, uBlock Origin supports this since last year.
They also support js injection, but only from a strictly maintained library of resources. Look for :style() and :inject() here: ​https://github.com/gorhill/uBlock/wiki/Static-filter-syntax They even support AdGuard filters format.

[Lain_13]

Since now we have framework for -abp- pseudoclasses in new #?# hiding filters then this could be implemented as pseudo-class :-abp-style(). Also, check #242 comment 34 for one special behavior.

[Lain_13]

Any chance this will be ever implemented? Dmitry (current RU AdList maintainer) already actively supports a list with styles for uBlock Origin which I generated from RU AdList CSS Fixes (relatively small list with ~650 filters, some of which applies to >30 domains at once). I still updating CSS Fixes style on userstyles.org from it, though, since there are ~68000 users right now and ~1000 installations per week. Besides, Adguard also supports cosmetic CSS filters.

At this point lack of CSS filters slowly turns into a hindrance. As for example recently I've installed Firefox on my phone, but instead of ABP I've used uBO since I don't need a separate extension for styles on my phone and I doubt anyone really want to since mobile platforms usually are limited in resources.

[greiner]

Replying to Lain_13:

At this point lack of CSS filters slowly turns into a hindrance. As for example recently I've installed Firefox on my phone, but instead of ABP I've used uBO since I don't need a separate extension for styles on my phone and I doubt anyone really want to since mobile platforms usually are limited in resources.

The ​Chrome Web Store policy states:

An extension must have a single purpose that is narrow and easy-to-understand.

Therefore it'd be great if we could narrow down the requirements instead of allowing any kind of CSS injection - also to avoid any security issues that come with it.

Based on the 540 styles in RU AdList CSS Fixes, I've compiled the following list containing the top 20 CSS properties it uses. The list below also includes a subset of values to exclude highly specific values such as background: radial-gradient(ellipse at center,#4e4e4e 0%,#1a1a1a 100%):

• 65x display
  • 63x none
• 54x margin-top
  • 45x 0
• 45x background
  • 18x none
• 43x padding-top
  • 32x 0
• 39x pointer-events
  • 25x none
  • 14x auto
• 35x height
  • 8x auto
  • 6x 0
• 30x top
  • 16x 0
  • 3x -10000px
• 27x width
  • 8x 100%
  • 7x auto
  • 3x 0
• 26x position
  • 16x fixed
  • 4x relative
  • 3x absolute
  • 3x static
• 22x background-image
  • 21x none
• 16x overflow
  • 14x auto
• 15x cursor
  • 14x auto
• 13x transform
  • 12x scale(0)
• 11x padding
  • 8x 0
• 11x background-color
• 10x left
  • 5x 0
  • 3x -10000px
• 8x margin-left
  • 6x 0
• 7x opacity
  • 7x 0
• 5x max-width
  • 2x none
• 5x max-height

Based on that I can say that display: none (63x), {margin,padding}{-{bottom,left,right,top}}: 0 (59x margin, 45x padding), background{-image}: none (39x) and pointer-events: none (25x) already make up over 40% of all styles.

While display: none is already supported anyway, it seems reasonable to add support for margin/padding for cleaning up left-over whitespaces. I'm not sure what background and pointer-events styles are useful for though so any information on that would be appreciated.

[mapx]

see other examples in ublock filters (search for :style )
​https://github.com/uBlockOrigin/uAssets/blob/master/filters/filters.txt

[Lain_13]

display:none rules are there to hide hiding filters from sites which tends to look for stuff in the main list, so they may remain in CSS Fixes.

Pointer event and background styles are useful to remove stuff like branding. In particular cases when branding is slapped right at BODY or in the div between BODY and content and onclick event is used to handle clicks there. Pointer events will block any reaction on clicks there and background(-image): none will remove the image. Cursor:auto is usually somewhere near to remove that hand cursor if site owner cared to change it to indicate that branding is actually clickable.

Overflow filters are used to fix cases when there is a pop-over on a site which disables ability to scroll content because all the modern libs designed to show pop-overs hate to leave a scroll-bar on the side and hide it as soon as they can. >_<

Transform and opacity combined with position:fixed and negative left-top coordinates are primarily used as alternatives to display:none when site checks visibility of ad blocks. Transform:scale(0) is especially amazing at that.

Radial gradients and a few other complex styles are there just to restore look of the site. It would be nice to have, but not a major loss if it won't be supported. Even though I don't see how background not based on external resource could break security anyway.

[greiner]

Replying to mapx:

see other examples in ublock filters (search for :style )
​https://github.com/uBlockOrigin/uAssets/blob/master/filters/filters.txt

Thanks. I did the same analysis on the 90 styles I found in there and the styles I mentioned above only make up ~12% of it. Unfortunately, due to the small amount of styles, there isn't a lot I can make out of it.

Anyway, here are the top 5 properties:

• 19x display
  • 15x block
  • 3x initial
  • 1x none
• 16x overflow
  • 16x auto
• 15x height
  • 10x 1px
  • 3x auto
  • 1x 0
  • 1x initial
• 6x visibility
  • 4x collapse
  • 1x hidden
  • 1x visible
• 6x padding-top
  • 5x 0

Replying to Lain_13:

That's very helpful, thanks.

Pointer event and background styles are useful to remove stuff like branding. In particular cases when branding is slapped right at BODY or in the div between BODY and content and onclick event is used to handle clicks there. Pointer events will block any reaction on clicks there and background(-image): none will remove the image. Cursor:auto is usually somewhere near to remove that hand cursor if site owner cared to change it to indicate that branding is actually clickable.

I've created #6550 for hiding background image ads.

Overflow filters are used to fix cases when there is a pop-over on a site which disables ability to scroll content because all the modern libs designed to show pop-overs hate to leave a scroll-bar on the side and hide it as soon as they can. >_<

I wonder whether the extension could do this automatically. Do you have an example page with such a pop-over ad that causes this issue when hidden?

Transform and opacity combined with position:fixed and negative left-top coordinates are primarily used as alternatives to display:none when site checks visibility of ad blocks. Transform:scale(0) is especially amazing at that.

I've created #6551 for providing more options to hide elements.

Radial gradients and a few other complex styles are there just to restore look of the site. It would be nice to have, but not a major loss if it won't be supported. Even though I don't see how background not based on external resource could break security anyway.

Agreed. From what I see, gradients are only used on two domains and providing the ability to specify background colors may be sufficient.

Is this something that users are complaining about or is this more of a personal preference?

[Lain_13]

I wonder whether the extension could do this automatically. Do you have an example page with such a pop-over ad that causes this issue when hidden?

Add filter:
##._top_990x70, .jqmOverlay, .jqmWindow, .megaShadow, div[class^="GbShadow"]
Visit:
​https://www.zdrav.ru/articles/4293658285-qsz-oshibki-pri-okazanii-onkopomoshchi
Scroll around a bit. Maybe visit another article.
Should work on the other domains from this list too:
26-2.ru,arbitr-praktika.ru,budgetnik.ru,business.ru,cxychet.ru,dirsalona.ru,fd.ru,gazeta-unp.ru,gd.ru,gkh.ru,glavbukh.ru,golovbukh.ua,hr-director.ru,kadrovik01.com.ua,kdelo.ru,kom-dir.ru,law.ru,lawyercom.ru,menobr.ru,nalogplan.ru,pro-goszakaz.ru,pro-personal.ru,resobr.ru,rnk.ru,stroychet.ru,trudohrana.ru,ugpr.ru,zarplata-online.ru,zdrav.ru

Is this something that users are complaining about or is this more of a personal preference?

Depends. In case of 3dnews that is their own style. They just slap ads on top of it occasionally. Cases where anything but whitesmoke is used are also based on original background color usually found in the styles overridden by ads. And naruto-base.su previously used background image with a similar pattern in the header. So I just attempted to reproduce the effect without external images or large blocks of base64. Did so out of personal preference, though, since bare color didn't seem good. In some cases users specifically requested to restore at least a background color when blocking/hiding an image revealed some awful background color applied alongside with it.

Regarding rare properties. They may seem not important, but the problem is that in some cases you have to restore something that were there but were overridden with style which came with ads and breaks site when ad is gone. I mean things like z-index or float. That's rare, but happens.

[mjethani]

For what it's worth I think this could be done as part of script injection (see #6539). We could support both JS and CSS injections. Our "snippets library" could include handy CSS snippets that filter authors could apply in certain cases.

For example:

nnm.me#%#no-background(body),unset-cursor(body),zero-margins(#cntnt)

no-background, unset-cursor, and zero-margins would be CSS snippets bundled with Adblock Plus, the text in the parentheses would be the selectors.

The above would produce the rules:

body {
  background: none !important;
}
body {
  cursor: unset !important;
}
#cntnt {
  margin: 0 !important;
}

This would be less of an issue security-wise.

[greiner]

Replying to mjethani:

For what it's worth I think this could be done as part of script injection (see #6539). We could support both JS and CSS injections. Our "snippets library" could include handy CSS snippets that filter authors could apply in certain cases.

Sounds great and pretty much in-line what I was thinking of with #6550 and #6551.

Regarding the suggested syntax, I'd imagine it to be useful to be able to pass options (preferrably predefined) to any snippet to future-proof the syntax.

FYI: Here's an example of how uBlock does it:

• ​Filter
• ​Snippet

[Lain_13]

FYI: Here's an example of how uBlock does it:

And they don't bother to do that with CSS at all. They may or may not check :style() filters for external and url-encoded images (never tested), but they definitely not using snippets for that.

I also think that this would be overdoing it. In some cases I had to set very specific values for margins and padding and in other cases I want to change only margin-top to avoid breaking the other 3 values. Also, if you make a snippet for 1px height then developers of ABP detectors will just change check for height > 0 to height > 1. With ability to set a value identical to actual element's height generic methods won't work or will be harder to apply. So, you'll have to add at least one extra parameter for value to set and validate on top of that with different validator per snippet like 33px for height, 1px 0 0 7px for margin, or #4e3b7d or whitesmoke for color. Ability to use calc() in the dimension also might be useful in some cases. It isn't used anywhere yet, but it doesn't mean it won't be.

I think whitelist for properties in the style and validation of values of a few specific properties which actually could be dangerous in a parser for -abp-style() is a lot more sane than a ton of snippets with unique value validators per snippet. We probably don't want to add generic background property into a whitelist since implementing sane validator for everything it can do could be troublesome except for setting it to none, transparent or a color.

[mjethani]

Thanks for the inputs, folks.

I'm thinking a "snippet" should be a JS function that runs as an extension content script (i.e. injected via the tabs.executeScript API). It can take exactly one argument, which is a string. The string itself can be a stringified JSON object, e.g. {"elementId":"trckd","strict":true}. It may be up to each snippet to decide how to interpret the sole string argument (and document it of course so filter authors know what to pass).

For the use case being discussed here, there may be a snippet called css that takes the CSS code as its argument. It might be implemented like so:

function css(code)
{
  browser.tabs.insertCSS(tabId, {
    code,
    frameId,
    matchAboutBlank: true,
    cssOrigin: "user",
    runAt: "document_start"
  });
}

(Assuming tabId and frameId are global variables available to the snippet library as it's loaded, see ​draft implementation).

So then you would write a filter like so (syntax still under discussion):

example.com#%#css #foo { background: white } .bar { margin-top: 0 }

[Lain_13]

This would be the best. However, as I understand there should be style validation. Will it be done on the rules parser side in such case?

[mjethani]

If there's any validation for the CSS then it should be done in the snippet code itself (in the above example, it would be in the css JavaScript function). There are multiple reasons for this, one of them is that the snippets library is likely to be updated dynamically on a daily basis, which means if someone finds a security vulnerability then we can update the validation immediately instead of having to wait for a new release of Adblock Plus.

[greiner]

I understand that it would be much more convenient and powerful from a filter creation perspective to just forward CSS code but there are a couple of points I'd like to bring up to provide a bit more background:

1. Standards and browsers tend to evolve so, for instance, properties that are perfectly safe today may become unsafe when browsers introduce new value types or make other changes. Adding complexity by updating extension logic through a separate channel to address that makes me quite worried from a security perspective.

2. We need to ensure that any functionality we expose is related to content filtering because if people start using Adblock Plus for different purposes (e.g. styling websites) we may end up violating extension store policies.

3. It's natural that other elements on the site can be affected when blocking/hiding a specific element and it's relatively easy for website authors to make sure their site doesn't break when certain elements are not shown. However, when applying arbitrary styles, this becomes quite a bit more difficult to identify the issue, what's causing it and how to fix it.

All I'm saying is that we need to be extremely careful with this and I'd strongly suggest to avoid any kind of remote code injection/execution since the benefits seem to be in no relation to the potential risk.

[Lain_13]

1. Standards indeed do evolve, but they don't suddenly and silently creep up into existing browser versions. They are announced in browser updates and there are usually at least a few months before anything new will reach anyone but alpha/beta testers of that particular browser. Is one month not enough to release and update for ABP to alleviate potential problem?

2. JS snippets in general aren't "content filtering" since they won't just remove objects from a web pages. They will disrupt existing scripts functionality and provide fake API and similar things to either block ads or prevent detection. Even though the end result might be the same that is not "content filtering" at all. And since ABP will allow only strict subset of properties and will restrict some of the values of these properties that definitely wont turn it into a tool even remotely useful for styling purposes. It would be as useful and user-friendly as hammering a screw into a wall with a boot.

Also, we already have uBO and Adguard there, and they are providing such functionality for a while already and no-one banned them for that.

3. > it's relatively easy for website authors to make sure their site doesn't break when certain elements are not shown

That's when and if they actually care that their site won't break. As you know some are using scripts that DO break their site when ads are blocked. Also, since we are the reason why ad isn't available and site end up broken it's also our job to clean up after ourself and we can do that much faster than anyone else.

[greiner]

1) That's based on the assumption that (a) browser makers implement the standard correctly, (b) browsers don't have bugs and (c) we'll be able to identify such issues in time.

2) I think we agree that a subset of CSS is fine. What we disagree on is whether to pass valid CSS code and strip out parts we don't like or whether we build the CSS code ourselves from a limited set of rules based on the information we find in the filters.

Also, we already have uBO and Adguard there, and they are providing such functionality for a while already and no-one banned them for that.

I admit that the risk is low because it's quite subjective to determine what the "primary purpose" of a given extension is.

Therefore I'd say the bigger risk is the execution of remote code (i.e. "Add-ons must be self-contained and not load remote code for execution" ​AMO) because it's more straight-forward to assess.

3)

That's when and if they actually care that their site won't break. As you know some are using scripts that DO break their site when ads are blocked.

True.

Also, since we are the reason why ad isn't available and site end up broken it's also our job to clean up after ourself and we can do that much faster than anyone else.

I think we mean the same because I'd say that our job is to do content filtering without breaking the site and thereby provide the user with an uninterrupted user experience.

[mapx]

​https://adblockplus.org/forum/viewtopic.php?p=176562#p176562

[Lain_13]

Replying to greiner:

1) That's based on the assumption that (a) browser makers implement the standard correctly, (b) browsers don't have bugs and (c) we'll be able to identify such issues in time.

True, but unless someone find a way to do RCE with just CSS without external/inlined resources (which should be strictly blocked) that shouldn't be an issue. There should be at least some degree of trust to platform. And if someone will find a way to do that then everyone will be affected and a lot of sites will turn into potential attack vectors instead of just extensions in general and ABP in particular.

2) I think we agree that a subset of CSS is fine. What we disagree on is whether to pass valid CSS code and strip out parts we don't like or whether we build the CSS code ourselves from a limited set of rules based on the information we find in the filters.

Yes. I do agree that validating rules might be a bit harder, but we will be using the same CSS syntax instead of inventing our own. That means anyone capable of writing simple CSS will be able to make such rules without learning entirely new syntax just for one extension.

Therefore I'd say the bigger risk is the execution of remote code (i.e. "Add-ons must be self-contained and not load remote code for execution" ​AMO) because it's more straight-forward to assess.

I think this could be easily "circumvented" by strictly telling users that ABP includes internal subscription with executable JS code which is updated more often than extension itself and is necessary for some filters to work. Like it already does for AWRL and some other stuff. Probably even add an option to disable it since it isn't integral part of the extension in the first place and some users will want to have such option anyway. Yes, some filters won't work, but that will be entirely by user's choice. We already have Grease-/Tamper-/Violentmonkey on AMO and they are doing exactly what AMO strictly forbids to do. And with 3rd party code on top of that. It is just not essential for them to work and snippets shouldn't be either.

Now should or shouldn't disabling JS snippets break inlined CSS is entirely different question.

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.

[hfiguiere]

refiled as ​https://gitlab.com/eyeo/adblockplus/adblockpluscore/-/issues/210