AdguardTeam / tsurlfilter

AdGuard content blocking library
GNU General Public License v3.0
50 stars 12 forks source link

Support for uBO media queries #70

Open scripthunter7 opened 1 year ago

scripthunter7 commented 1 year ago

uBO now supports media queries. The tsurlfilter's converter does not support it yet.

Reference: https://github.com/gorhill/uBlock/wiki/Procedural-cosmetic-filters#subjectmatches-mediaarg

Examples:

FIXME:

Reference: https://kb.adguard.com/en/general/how-to-create-your-own-ad-filters#examples-9

@slavaleleka Do you have a better idea for the last case? 🙂

ameshkov commented 1 year ago

@maximtop @scripthunter7

Copy/pasting implementation ideas from the comment in a duplicate issue:

Possible implementations:

  1. Support it uBO-like, i.e. natively (parse the rule, check if there's :matches-media, take it into account when composing a rule).
  2. Via rules converter

Tbh, I am not sure which one is better.

Also, we should definitely file it to CoreLibs as well.

ameshkov commented 1 year ago

Regarding the converter way, I am not entirely sure about it.

Of course we could convert them into a simple CSS rule, i.e. ##.banner:matches-media(xxx) --> #$#@media xxx { .banner { display: none; } }. Is it the best way for a filters maintainer?

With how things done now element hiding rules with @media support cannot be implemented.

One more idea would be to introduce a new cosmetic modifier "media" and write those rules this way: [media="xxx"]##.banner

In this case the rules converter will do a simple ##.banner:matches-media(xxx) --> [media="xxx"]##.banner conversion.

scripthunter7 commented 1 year ago

One more idea would be to introduce a new cosmetic modifier "media" and write those rules this way: [media="xxx"]##.banner

In my opinion, it makes more sense to use the native CSS syntax at the engine level and convert the other rules.

With how things done now element hiding rules with @media support cannot be implemented.

It seems this is currently a very rare case and can be solved with a CSS rule:

#$#@media(media query list) { css selector list { display: none !important; } }

By the way, this case is a bit similar to when scriptlets get a path constraint (see https://github.com/AdguardTeam/tsurlfilter/issues/65). For example ##+js(scriptlet):matches-path(/path) -> [$path=/path]#%#//scriptlet('scriptlet')

ameshkov commented 1 year ago

In my opinion, it makes more sense to use the native CSS syntax at the engine level and convert the other rules.

I am more or less okay with both approaches.

IMO, [media="..."] modifier has the following advantages:

  1. Simplifies cosmetic rule parsing & validation, no need to handle @media() { { } } case.
  2. It will be easier to group rules by their media value, i.e. if there're multiple rules.

Both simplifications are really minor and can be easily achieved with the native CSS syntax as well.

On the other hand, using native CSS syntax has a very huge and clear advantage over that new modifier, we don't need to bother implementing anything new.

scripthunter7 commented 1 year ago

Well, if necessary, we can directly parse a CSS media query list (e.g. with CSSTree), so for me the modifier also seems like a good idea, and syntactically it is easy to use. For my part, I simply prefer native CSS, but your idea is more flexible:)

If I understand everything correctly, the following two rules are both valid on this way:

(Although in the case of CSS injections, the native CSS syntax seems more natural to me, and I don't see any complications in terms of parsing either)

And we can simplify #$#@media (min-width: 1000px) and (max-width: 2000px) { .element-to-hide { display: none !important; } } to [$media=(min-width: 1000px) and (max-width: 2000px)]##.element-to-hide

Other aspects:

ameshkov commented 1 year ago

Thank you!

Actually, there are only a few dozens of these rules in the known filter lists so probably bothering with implementing a new modifier is not feasible anyway.

What are the possible limitations? For example, does the Safari Content Blocker handle the media query?

In Safari these CSS rules are only applied via web extension, Safari Content Blocker does not provide media queries support.

Off-topic:

It seems we don't highlight it properly in the VS code extension.

scripthunter7 commented 1 year ago

It seems we don't highlight it properly in the VS code extension.

It has already been fixed, but we haven't released a new version of the extension since then. Linguist uses the improved version (last } tokenized fine):

#$#@media (min-width: 1px) { .ad { padding: 1; } }