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

ABP new filter not working on uBO #2793

Closed
kaypoush opened this issue Jul 18, 2017 · 63 comments
Closed

ABP new filter not working on uBO #2793

kaypoush opened this issue Jul 18, 2017 · 63 comments

Comments

@kaypoush
Copy link

Hello,

https://adblockplus.org/development-builds/new-syntax-for-advanced-element-hiding-rules

New filter -abp-has doesn't work on uBO.
`
Test :

https://yggtorrent.com/torrent/filmvid%C3%A9o/s%C3%A9rie-tv/23187-game+of+thrones+s07e01+vostfr+1080p+web+h264-extreme+mkv

Filter :

yggtorrent.com#?#TR > TD > TABLE > TBODY > TR:-abp-has(A[href$="/go_vpn"])

Source :

https://forums.lanik.us/viewtopic.php?f=91&t=37526

Config :

Chrome + uBO + Default (+ Liste FR)

Thanks.

@gorhill
Copy link
Owner

gorhill commented Jul 18, 2017

Should be trivial to fix at first glance.

Note: I tried the filter in ABP. On my side, it works if refreshing the already loaded test page, however it does not work if opening in a new tab.

@mapx-
Copy link

mapx- commented Jul 18, 2017

I filed a new ticket
https://issues.adblockplus.org/ticket/5425#ticket

@gorhill
Copy link
Owner

gorhill commented Jul 20, 2017

@mapx- Looks like a timing issue: if you just click refresh button, it works; If you shift-click refresh button (thus bypassing the browser cache), it doesn't work.

@mapx-
Copy link

mapx- commented Jul 21, 2017

Yes, ..sometimes even F5 still leave the ad unhidden.

@kasper93
Copy link
Contributor

kasper93 commented Jul 29, 2017

I've already sent pull request with support for -abp-has() filters 26 days ago. #2755

@gorhill
Copy link
Owner

gorhill commented Sep 28, 2017

Fixed with 8559669.

Additionally, compatibility with Adguard's :contains() has been added.

@gorhill gorhill closed this as completed Sep 28, 2017
@gorhill gorhill reopened this Sep 30, 2017
@gorhill
Copy link
Owner

gorhill commented Sep 30, 2017

I want to do more work on this.

Specifically, I want uBO to normalize the raw filter so that it can properly handle duplicates such as:

facebook.com##div[id^="hyperfeed_story_id_"]:has(a:has-text(Chartered))

And

facebook.com#?#div[id^="hyperfeed_story_id_"]:-abp-has(a:-abp-contains(Chartered))

Which essentially accomplishes the same thing.

uBO will normalize procedural filters to properly discard duplicates -- especially given how procedural cosmetic filters are more CPU intensive. For instance, the above will be normalized to:

facebook.com##div[id^="hyperfeed_story_id_"]:if(a:has-text(Chartered))

gorhill added a commit that referenced this issue Sep 30, 2017
@gorhill gorhill closed this as completed Sep 30, 2017
@joey04
Copy link

joey04 commented Oct 2, 2017

@gorhill have a question about these new changes.

tvguide.com##.listings-tab-link.active:matches-css-after(opacity: 1)

I see atomized as:
untitled

Is the atomized /1/ by design? It still works fine, but I don't recall seeing it that way in the Logger before. (This is the only procedural rule I have.)

@gwarser
Copy link
Contributor

gwarser commented Oct 2, 2017

@joey04
Copy link

joey04 commented Oct 2, 2017

Thanks for the info, gwarser.

But I can confirm it has indeed changed. I forgot earlier I have GChrome with stock uBO rev 1.14.8. This is what my rule compiles to there:

untitled

So why the change to regex /1/ in the 1.14.11 beta?

@gwarser
Copy link
Contributor

gwarser commented Oct 2, 2017

Apparently part of normalization

@gorhill gorhill reopened this Oct 2, 2017
@gorhill gorhill closed this as completed in bd18fe3 Oct 4, 2017
@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

Just use ##img.

@mapx-
Copy link

mapx- commented Nov 24, 2017

yes, it works with / without ?

@mapx-
Copy link

mapx- commented Nov 24, 2017

uBo supports ABP and its own syntax, ABP ...doesn't

@mapx-
Copy link

mapx- commented Nov 24, 2017

yes and uBo is supporting it.

@mapx-
Copy link

mapx- commented Nov 24, 2017

ABP doesn't support uBo implementation.

@mapx-
Copy link

mapx- commented Nov 24, 2017

uBo supports the ABP syntax for :-abp-has / :-abp-contains with / without ?
ABP does not support :has / :has-text (uBo standard)

Just do your tests and report back if you have real doubts

@mapx-
Copy link

mapx- commented Nov 24, 2017

Provide all the info: page link, what do you want to hide, eventually upload a screenshot indicating what do you want to hide, used filter, browser version

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

Isn't ABP's syntax #?#?

My understanding was that this syntax was to be used only for procedural filters. I just tried it with ABP, and you are right, example.com#?#img works. So now I have no idea ABP went with #?#if it can be used as a replacement of ##. I am pretty sure however that no filter list maintainer is going to use #?#img instead of ##img. Maybe the fix is required on ABP 's side? I can't read their mind, I don't know why they went with #?#.

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

How to chain :nth-of-type(2) and :has-text()?
These filters doesn't work perfect yet.

Can you open a new issue about this? I used regexes internally to parse procedural filters and I might have to move on parsing in a more reliable way without having to rely on regexes.

@Halibut80
Copy link

@gorhil, I have a long-standing question, but I'm not sure, this is by design or I should also open issue about this:
These sort of filters is also does not work:

example.com##div:if(> div:has-text(Some)) + div
example.com##div:if(> div:has-text(Some)) ~ div
example.com##div:has(> div) + div

etc.

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

@Halibut80 Not supported. Open an issue if you wish, though I would work on this if I become convinced there is a real actual need in the wild rather than just a nice-to-have one.

@Halibut80
Copy link

Well, i have some real case for this sort of rules, but they only for... hm, for personal easement (ughm, sorry, I don't have words to clear explain my phrase (my English is very bad)), and they also can be done with xpath, but last example looks like valid css selector and should work, or I'm wrong?

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

The parser for procedural cosmetic filters does not expect trailing stuff past the last parenthesis, so that fails as well even if it's supposed to be a valid CSS4 selector. Really I just went with the simpler implementation at first for what was needed as per what was seen in the wild at the time.

@Halibut80
Copy link

Ok, thank you. This is a very minor problem, and can be done in a different way, so I will not open a new bug and waste your time.

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

subscriptions are written for ABP, so you have to support ABP syntax

No filter list maintainers will use #?# for a normal filter. The fact that it works is incidental. I don't have to support something which is never meant to be used this way.

@okiehsch
Copy link
Contributor

spielaffe.de#?##advertising_billboard
spielaffe.de#?##app_advertising_skyscraper_right
kino.de#?#.banner

part of EasyListGermany

@okiehsch
Copy link
Contributor

cnet.com#?#.ad-leader-top
cnet.com#?#.ad-mpu-plus-bottom
cnet.com#?#.ad-mpu-plus-middle
cnet.com#?#.ad-mpu-plus-top
cnet.com#?#.ad-nav-ad
cnet.com#?#.ad-replay-wide-top

part of EasyList

@okiehsch
Copy link
Contributor

The syntax is supported for advanced selectors, I don`t know why EasyList started to use it for "normal"
filters.

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

Wait, these are properly handled in uBO, I just stepped into the code. I just tried #?#img again and it worked fine. So no issue in the end.

a

@okiehsch
Copy link
Contributor

okiehsch commented Nov 24, 2017

@gorhill
gdfg
seems to work anyway.
Edit: Sorry, missed your comment.

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

My mistake, I should have demanded reproducible steps. I can't answer if I can't reproduce exactly what you tried.

@mapx-
Copy link

mapx- commented Nov 24, 2017

well, I tested this morning (and works with / without ?) and already answered to the user and asked a real test example :)
#2793 (comment)

@okiehsch
Copy link
Contributor

Sorry, I missed that.

@mapx-
Copy link

mapx- commented Nov 24, 2017

@ololoe if your filter is exactly #?#img it does not work because in that syntax ABP expects to have non-generic filters.

You need example.com#?#img

@mapx-
Copy link

mapx- commented Nov 24, 2017

github.com#?#img tested on my github profile, is working fine hiding my avatar (chrome)

working in FF 57 too

@mapx-
Copy link

mapx- commented Nov 24, 2017

yes, see above, working in FF 57 too + uBo (last dev build)

@okiehsch
Copy link
Contributor

okiehsch commented Nov 24, 2017

#?#img will not work if you enter it in uBO's interactive element picker mode.
Why should it? If you are using uBO why would you use #?#.

@okiehsch
Copy link
Contributor

Because it is an Adblock/Plus specific filter, I still don't know why you would use #?# instead of ## if you are trying to create a filter with uBlock Origins element picker.
I think it is sufficient for uBO to properly handle these Adblock/Plus specific filters if you add them to your filter list.

@okiehsch
Copy link
Contributor

Add github.com#?#img to your filter list, click apply and then reload this page.

@mapx-
Copy link

mapx- commented Nov 24, 2017

@ololoe I already answered you twice. Yes, it works, add the filter in "my filters".

@gorhill
Copy link
Owner

gorhill commented Nov 24, 2017

you even don't accept as a bug?

Right, I don't accept as a bug, because the workaround is: don't use ? in the element picker, you don't have to.

Repository owner locked and limited conversation to collaborators Nov 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants