Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cosmetic filter does not always hide elements #3127

Closed
7 of 9 tasks
JackMauro opened this issue Feb 17, 2024 · 9 comments
Closed
7 of 9 tasks

Cosmetic filter does not always hide elements #3127

JackMauro opened this issue Feb 17, 2024 · 9 comments
Labels
bug Something isn't working fixed issue has been addressed

Comments

@JackMauro
Copy link

JackMauro commented Feb 17, 2024

Prerequisites

  • I verified that this is not a filter list issue. Report any issues with filter lists or broken website functionality in the uAssets issue tracker.
  • This is NOT a YouTube, Facebook or Twitch report. These sites MUST be reported by clicking their respective links.
  • This is not a support issue or a question. For support, questions, or help, visit /r/uBlockOrigin.
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue.
  • The issue is not present after disabling uBO in the browser.
  • I checked the documentation to understand that the issue I am reporting is not normal behavior.

I tried to reproduce the issue when...

  • uBO is the only extension.
  • uBO uses default lists and settings.
  • using a new, unmodified browser profile.

Description

Cosmetic filters are not always applied correctly.
Filters like this:
hwupgrade.it,punto-informatico.it,tomshw.it,telefonino.net##[href*="super-sconto"]
appears in yellow in logger, marked as DOM filters, but the element is not hidden in the page.

ublock

this is the list of custom filters about that site. I have also tried to load them as "my filters", but still doesn't hide the links.
hwlist.txt

Thank you

A specific URL where the issue occurs.

https://hwupgrade.it/

The list that contains these filters:
https://jack.logicalsystems.it/homepage/AdBlockPlus.txt

Steps to Reproduce

  1. add the custom filter list https://jack.logicalsystems.it/homepage/AdBlockPlus.txt
  2. open the logger window
  3. visit https://hwupgrade.it/

Filters will appear yellow in logger but the elemts will not be hidden

Expected behavior

This is the same page using adblock plus and same list:
adblock

i expect that cosmetic filter hides all matching tags

Actual behavior

ublock

uBO version

1.55.0

Browser name and version

Chromium Version 109.0.5414.120 (Official Build, ungoogled-chromium) (32-bit)

Operating System and version

Windows 7

@gorhill
Copy link
Member

gorhill commented Feb 18, 2024

There is a bad cosmetic filter in that list:

hwupgrade.it##DIV[align="center"] > TABLE[width="550"] TD[bgcolor="#F6F6F6\a "]

If you create an exception for that filter (or remove it from the list), all works fine:

#@#DIV[align="center"] > TABLE[width="550"] TD[bgcolor="#F6F6F6\a "]

The bad part is the \a character, it's being converted to newline by the compiler, resulting in this filter after compilation:

hwupgrade.it##DIV[align="center"] > TABLE[width="550"] TD[bgcolor="#F6F6F6\n "]

Which is an invalid selector as seen when entering the following in dev console:

document.querySelector('DIV[align="center"] > TABLE[width="550"] TD[bgcolor="#F6F6F6\n "]')
Uncaught DOMException: Failed to execute 'querySelector' on 'Document': 'DIV[align="center"] > TABLE[width="550"] TD[bgcolor="#F6F6F6
 "]' is not a valid selector.

However the cosmetic filter compiler should catch such invalid filter and discard it, but it didn't. This needs to be fixed.


Note that there are many other invalid cosmetic filters in that list which are caught and discarded by uBO. If you open the list with the eye icon, uBO highlights those invalid filters. Example:

image

ABP won't notify you about such invalid cosmetic filters, and will actually even inject them into the document. However in ABP each cosmetic filter is injected in its own CSS declaration, such that they won't break other valid cosmetic filters. uBO injects all cosmetic filters into as few CSS declaration as possible such that an invalid cosmetic filter will break other cosmetic filters in the same CSS declaration. This works fine unless the filter compiler fails to detect an invalid cosmetic filter such as the case here.

@gorhill
Copy link
Member

gorhill commented Feb 18, 2024

There is a bad cosmetic filter in that list

Correction: as per CSS character escape sequences, the selector is valid:

To include a newline in a string, use an escape sequence representing the line feed character (U+000A), such as "\A".

The issue is that I didn't expect CSSTree (the tool uBO uses to compile cosmetic filters) to cause newlines to end up in parsed selector data. The fix is for uBO to be prepared for this to happen.

@gorhill
Copy link
Member

gorhill commented Feb 18, 2024

Fix will be in next dev build.

I haven't decided yet if I should publish a revision to last stable release.

@gwarser gwarser added the bug Something isn't working label Feb 18, 2024
@gwarser
Copy link

gwarser commented Feb 18, 2024

assets.json:

assets.json_resources/GRC-0_gr cy Greek AdBlock Filter.txt.zst:skai.gr##.skin.news[href="https://www.facebook.com/NAIstinEllada\a "]
assets.json_resources/POL-0_pl Oficjalne Polskie Filtry do uBlocka Origin.txt.zst:forum.vwgolf.pl##A[style="position:absolute;bottom:0.4em;text-transform:uppercase;font-size:80%;right:1em;\a           background:#de8c2f;color:#e6ebf2;padding:3px 7px; display:block"]

[some lists were old]

Other lists:

filterlists.com_resources/51_Greek AdBlock Filter.txt.zst:skai.gr##.skin.news[href="https://www.facebook.com/NAIstinEllada\a "]
filterlists.com_resources/99_YousList.txt.zst:cyberciti.biz##div.post_content>div[data-amp-original-style="text-align: center;\a     background-color: rgb(247, 247, 247);\a     border: 1px solid rgb(243, 243, 243);\a     width: 100%;\a     padding-bottom: 18px;\a     border-radius: 7px;     \a     margin-top: 20px;        \a     clear: both;"]
filterlists.com_resources/99_YousList.txt.zst:cyberciti.biz##div.post_content>div[style="text-align: center;\a     background-color: rgb(247, 247, 247);\a     border: 1px solid rgb(243, 243, 243);\a     width: 100%;\a     padding-bottom: 18px;\a     clear: both;"]
filterlists.com_resources/217_Adblock Persian List.txt.zst:##DIV[style="position:fixed; right:86%; top:83%; float:left; border:1px #999 solid; padding:7px; background:#ffffff; width:150px; border-radius: 10px;\a -moz-border-radius: 10px; font-family:tahoma; font-size:11px; "]
filterlists.com_resources/217_Adblock Persian List.txt.zst:downloadkade.com##DIV[style="position:fixed; right:91%; top:82%; float:left; border:1px #999 solid; padding:7px;  width:65px; border-radius: 10px;\a -moz-border-radius: 10px; font-family:tahoma; font-size:11px; "]
filterlists.com_resources/223_Official Polish Filters for AdBlock, uBlock Origin & AdGuard.txt.zst:forum.vwgolf.pl##A[style="position:absolute;bottom:0.4em;text-transform:uppercase;font-size:80%;right:1em;\a           background:#de8c2f;color:#e6ebf2;padding:3px 7px; display:block"]
filterlists.com_resources/227_Acceptable Ads.txt.zst:portail.free.fr#@#div[style="margin: 0 auto;width: 1000px;background-color:#fff;height:300px;\a   border-bottom: 4px solid #2d98e0"]
filterlists.com_resources/251_Filtros Nauscopicos.txt.zst:eureka-startups.com##IMG[style="width:300px; \a \a height:250px; border:0px;"]
filterlists.com_resources/260_Fanboy's Polish.txt.zst:cyber-sport.us##td[style="height:\9 25px; border-top:\9 1px solid rgb(153, 153, 153); border-right:\9 none; border-bottom:\9 none; border-left:\9 1px solid rgb(153, 153, 153); background-color:\9 rgb(0, 0, 0); color: rgb(207, 189, 140);\a text-align:\9 center;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:electronicproducts.com##div[style="width: 741px; height: 90px; margin: 5px auto 15px;\a \9 \9 padding: 0px; background-color: rgb(255, 255, 255);"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:kicknews.net##div[style="border: solid blue 0px; \a width: 996px; \a height: 86px; \a margin: 0 auto; \a position: relative; -webkit-box-shadow:  0px 0px 2px 2px rgba(6, 6, 6, 0.5); \a box-shadow:  0px 0px 2px 2px rgba(6, 6, 6, 0.5);\a -webkit-border-radius: 8px 8px 0px 0px;\a border-radius: 4px 4px 0px 0px;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:mp3raid.com##td[style="background:url('http://images.mp3raid.com/mobile.gif');\a background-repeat:no-repeat;height:55px;padding-left:0px;border-right:1px solid #8FB9D0;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:ontopmag.com##div[style="padding-top: 10px; background-color: #575757; height: 90px;\a            width: 728px; margin: auto auto;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:profilecanada.com##div[style="width:260px; height:240px; \a                         float:left; padding-left:8px; text-align:left;\a                          font-family:Arial, Helvetica, sans-serif; \a                     font-size:12px; font-weight:normal; color:#2d2d2d;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:raysindex.com##img[style$="text-align: center; cursor: \a \a pointer; width: 728px;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:roxigames.com##div[style="width:728px;height:90px;\a border:1px solid blue;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:slyck.com##div[style="width:295px; border: 1px solid #DDDDDD; text-align: center;\a  background: #FFFFFF; padding: 5px; font:12px verdana;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:teamforge.net##div[style="display: block; margin: 0.5em 1.5em; padding: 1em; border-style: solid;\a \9 border-color: #F0FFFF; border-width: 3px; background-color: #F0F0F0; font-size: xx-small; text-align: left;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:uncrate.com##div[style="width: 598px; height: 300px; padding: 15px; margin: 0 0 30px 0;\a \9 background: #FFF;\a \9 border: 1px solid #999999;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:xtremedotnettalk.com###showimage[style="position:absolute;width:700px;left:300px;top:800px;border:1px solid \a \a #cccccc;text-align:center;z-index:999;background:#fff"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:yuku.com##div[style="width: 300px; margin-left: 5px; margin-bottom: 5px;\a \9 \9 \9 \9 \9 \9 \9 \9 border: 1px solid #CCC; padding: 3px;float:right;"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:bunny4.com##div[style="width: 728px; height: 90px; border: 1px solid rgb(223, 223, \a 223);"]
filterlists.com_resources/263_Fanboy's IsraelList.txt.zst:slutleech.com##div[style="display:block;margin:10px auto;text-align:center;background: yellow;\a padding-top: 10px;\a border-radius: 10px;"]
filterlists.com_resources/375_All-in-One Customized Adblock List.txt.zst:briefing.com##table[style="vertical-align: top; text-align: left;\a \9             background-color: White; width: 100%; padding: 10px;"]
filterlists.com_resources/375_All-in-One Customized Adblock List.txt.zst:designtaxi.com##div[style="\a     margin-bottom: 15px;\a "]
filterlists.com_resources/375_All-in-One Customized Adblock List.txt.zst:polomarket.pl,szyj.pl##div[style="padding:5px 15px;padding-top:10px;width:auto;font-family:arial;\a     margin:0px auto;color:#5E5149;text-align:center;text-shadow:0 1px 0 #fff;"]
filterlists.com_resources/375_All-in-One Customized Adblock List.txt.zst:sedaily.com##div#container>div[style="width: 1196px; height: 250px;\a                                             clear:both;\a                                             margin: 0 auto;\a                                             display: block;\a                                             margin-top: 40px;\a                                             margin-bottom: -70px;\a                                             border: 1px solid #ced2d7;"]
filterlists.com_resources/375_All-in-One Customized Adblock List.txt.zst:tgd.kr##div.frame>div[style="background:#fff;    background: #fff;\a     padding: 20px 0;\a     margin-top: -14px;\a     text-align: center;"]

@gwarser gwarser added the fixed issue has been addressed label Feb 18, 2024
@gwarser
Copy link

gwarser commented Feb 18, 2024

Each affected list must be updated for fix to work? I tried with https://hwupgrade.it/ and after uBO update I had to also update the list. Then I tried https://forum.vwgolf.pl/ and here also I had to update POL list (elements not hidden and DOM inspector did not work - was empty). Was it always like this in such cases?


Maybe I did not think about it before - just purged caches :)

@gwarser gwarser closed this as completed Feb 18, 2024
@gorhill
Copy link
Member

gorhill commented Feb 18, 2024

Each affected list must be updated for fix to work?

Yes, they need to be re-compiled.

@JackMauro
Copy link
Author

Note that there are many other invalid cosmetic filters in that list which are caught and discarded by uBO. If you open the list with the eye icon, uBO highlights those invalid filters.

I have fixed almost all errors in the list, but if anyone needs the old one for testing purposes, i renamed it to AdBlockPlusBroken.txt

Thank you for the quick response and fix!

@u-RraaLL
Copy link
Contributor

uBO injects all cosmetic filters into as few CSS declaration as possible such that an invalid cosmetic filter will break other cosmetic filters in the same CSS declaration

Would it make sense for uBO to encase selectors in a forgiving selector list such as :is() to void such breakages?

Unless I'm missing something, the minimum browser versions for uBO are already beyond what it requires (due to the :not() selectors list).

@gorhill
Copy link
Member

gorhill commented Feb 18, 2024

:is() was found to be slow, best to avoid. We don't need to overdo it, it took a long time for the case here to be reported, suggesting invalid selectors being injected in the DOM is a rare thing. This will be even rarer with the current fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed issue has been addressed
Projects
None yet
Development

No branches or pull requests

4 participants