tsurlfilter icon indicating copy to clipboard operation
tsurlfilter copied to clipboard

Support for uBO media queries

Open scripthunter7 opened this issue 2 years ago • 7 comments

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:

  • example.com###target-3 > .target-4:matches-media((min-width: 1920px) and (min-height: 930px)):style(color: red !important)
    

    should be converted to

    example.com#$#@media (min-width: 1920px) and (min-height: 930px) { #target-3 > .target-4 { color: red !important; } }
    
  • github.com##pre:matches-media(print):style(white-space:pre-line !important;)
    

    should be converted to

    github.com#$#@media (print) { pre { white-space:pre-line !important; } }
    

FIXME:

  • This is typically an element hiding rule

    example.com###target-1 > .target-2:matches-media((min-width: 800px))
    

    and perhaps it can be converted to

    example.com#$#@media (min-width: 800px) { #target-1 > .target-2 { display: none !important; } }
    

    but I'm not sure about that

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? 🙂

scripthunter7 avatar Dec 17 '22 19:12 scripthunter7

@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 avatar Apr 14 '23 14:04 ameshkov

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.

ameshkov avatar Apr 14 '23 14:04 ameshkov

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')

scripthunter7 avatar Apr 14 '23 17:04 scripthunter7

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.

ameshkov avatar Apr 16 '23 19:04 ameshkov

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:

  • #$#@media (min-width: 1000px) and (max-width: 2000px) { body { padding: 0 !important; } }
  • [$media=(min-width: 1000px) and (max-width: 2000px)]#$#body { padding: 0 !important; }

(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:

  • Should this media modifier have an effect on any other rules in addition to the element hiding and CSS injection rules? Does that make sense? app, path and domain are generic, but media cannot affect HTML filtering rules, for example
  • What are the possible limitations? For example, does the Safari Content Blocker handle the media query?
  • Statistics:
    • uAssets:
      • CSS injection + media queries: only 3 rules
      • element hiding + media queries: 0 rules
    • AdGuard Filters:
      • CSS injection + media queries: only 22 rules
      • CSS injection as element hiding (display: none): 1 rule

scripthunter7 avatar Apr 16 '23 19:04 scripthunter7

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.

ameshkov avatar Apr 16 '23 20:04 ameshkov

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; } }

scripthunter7 avatar Apr 16 '23 21:04 scripthunter7