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 issue #2390: "Create a new filter class for CSS property filters" #139

Closed
gorhill opened this issue Apr 27, 2015 · 22 comments
Closed

ABP issue #2390: "Create a new filter class for CSS property filters" #139

gorhill opened this issue Apr 27, 2015 · 22 comments
Labels

Comments

@gorhill
Copy link
Owner

gorhill commented Apr 27, 2015

It seems ABP is creating a new class of cosmetic filters:

#2390: Create a new filter class for CSS property filters

Will need to keep track when this become reality, and then eventually implement support for this in uBlock.

My current understanding of this new class of cosmetic filters is to gain the ability to run a regex against the value of the style attribute of a DOM element.

@massar
Copy link

massar commented Jul 17, 2015

FYI: Apparently there was some code commited:
https://hg.adblockplus.org/adblockplus/rev/f99ad521dd5f

@gorhill
Copy link
Owner Author

gorhill commented Jul 17, 2015

Just to be clear, I don't take code from ABP -- I implement my own code according to spec.

@massar
Copy link

massar commented Jul 17, 2015

@gorhill I am well aware (but yes, others might not), your code does not contain remarks like "CSS property filters are inefficient" :)

I could not quickly find a spec unfortunately for this new setup.

Being able to allow CSS (for stuff like colors/locations) when that CSS is loaded from a domain enabled in dynamic filtering would be a nice thing; I guess though that this feature would help a lot already as then odd css for loading fonts etc can easily be filtered out.

@lewisje
Copy link

lewisje commented Feb 9, 2016

This new syntax was announced in December and just recently implemented in stable builds of ABP: https://adblockplus.org/development-builds/new-css-property-filter-syntax

@gorhill
Copy link
Owner Author

gorhill commented Jun 26, 2016

I commented in issue #1752:

I do worry about the overhead introduced by -abp-properties

To confirm/infirm whether I worry needlessly, I profiled a page (timings are for reloading the page 10 times) where -abp-properties is used, here is the results (ABP top, uBO bottom, both using EasyList, EasyPrivacy + EasyList Germany):

a

The yellow-highlighted lines are the top contributed overhead by the content scripts of the extension. As expected, -abp-properties adds a significant overhead to page load (again, keep in mind those timings are for 10 page loads -- divide by 10 if you want per-page load timings), as seen by the CSSPropertyFilters.onLoad() timings (and in the current case, those filters seemingly didn't accomplish anything, the page still loaded those annoying ads). I observe the overhead introduced by -abp-properties handling code is not present if there is no -abp-properties filters for the page.

@TheFinalCut83
Copy link

TheFinalCut83 commented Jun 27, 2016

This is strange, but when I checked your example with focus.de (Cyberfox 47.0.1, same filters as yours), uBO seems to have missed that ads at the top.
1
I've reloaded the page with cache and without at least 5 times.
ABP, same filters.
3
German IP, of course
2
But yes, not a VPN, but browser plugin Zenmate, maybe such result because of this.

@gorhill
Copy link
Owner Author

gorhill commented Jun 27, 2016

@TheFinalCut83: because they fixed the -abp-properties filters just hours ago (overhead is still the issue, which is what my comment was about). The site is properly filtered with uBO, tested on FF47 and Seamonkey using a .de VPN. It's as if you do not have uBlock filters enabled.

@IDKwhattoputhere, @TonyTough, do you get the same thing for uBO as @TheFinalCut83? All works fine from my side.

@ghost
Copy link

ghost commented Jun 27, 2016

Cyberfox:
https://cloud.githubusercontent.com/assets/16030872/16380211/f50e7b3c-3c75-11e6-96e1-6d902d00278e.jpg
Sometimes I get no, sometimes I get some and other times I get all the banners (after reloading, clearing cache doesn't seem to matter).
https://cloud.githubusercontent.com/assets/16030872/16380217/fbef477e-3c75-11e6-9ed0-2f737441923c.jpg
Firefox:
https://cloud.githubusercontent.com/assets/16030872/16380508/c3c661a0-3c77-11e6-8381-3e4735082457.jpg
Same behaviour except that there's always a delay of multiple seconds before the banners are inserted (after the page has finished loading) which I didn't notice in Cyberfox.
https://cloud.githubusercontent.com/assets/16030872/16380514/cd0d161e-3c77-11e6-93d3-72f7f1c71271.png

Edit: I seem to have misunderstood. By

both using EasyList, EasyPrivacy + EasyList Germany

I thought you meant only those filter lists. I tried again with default lists + EasyList Germany and now I never get ads (tested Cyberfox and Firefox).

@gorhill
Copy link
Owner Author

gorhill commented Jun 27, 2016

I thought you meant only those filter lists

For uBO, I meant default filter lists + EasyList Germany, for ABP I meant EasyList + EasyList Germany + EasyPrivacy minus "Acceptable ads".

@TonyTough
Copy link

TonyTough commented Jun 27, 2016

For me all is OK on focus.de with ublocks standard filters + Easylist Germany and Firefox 47 (German IP).
I use focus.de several times a day.

@TheFinalCut83
Copy link

Yes, with default filter lists + EasyList Germany all is OK.

@ryanbr
Copy link
Contributor

ryanbr commented Jan 10, 2017

Recently I've been tackling a few base64 encoded ad sites on onwatchseries.to (and mirror sites). Using

onwatchseries.to##[-abp-properties='data:'] onwatchseries.to##[-abp-properties='base64']

Which doesn't seem to work well in uBo. I'm pretty sure you can inject scripts etc, but given the popularity of base64 encoded ads, -abp-properties support would be helpful for a quick and dirty fix.

Related commits:
easylist/easylist@129c248
easylist/easylist@fb484f3
easylist/easylist@f664b6a

@gorhill
Copy link
Owner Author

gorhill commented Jan 10, 2017

-abp-properties support would be helpful for a quick and dirty fix.

As said, it's performance-wise scary. Each rule in each stylesheet present on the page must be iterated, and for each of these rules, a string version of all its own CSS properties, sorted, must be constructed and the result tested against a regular expression derived from the filter. Repeat for all single -abp-properties filter.

What I am seeing on onwatchseries.to after updating EasyList to get the filters above is that the exception filters @@||bambergerkennanchitinous.com^$script,domain=[...] is at the root of these ads, it allows the request http://bambergerkennanchitinous.com/.adframesrc. to go through unblocked. Why not just remove this exception filter?

The real issue from uBO's point of view is ABP's limitations forcing the use of such exception filters in EasyList, so aside -abp-properties being a performance concern, it's the wrong fix: it fixes the symptoms, the root issue is those exception filters created for the sake of ABP.

@uBlock-user
Copy link
Contributor

uBlock-user commented Jan 10, 2017

If you pick gorillavid.in as the source on onwatchseries.to, it now tries to load popups using ddata: as the protocol FYI

Logger - https://i.gyazo.com/93af9a3c8d65e4ed6e65c69e4621bea3.png

Is that even possible using ddata: as the protocol ?

@lewisje
Copy link

lewisje commented Jan 10, 2017

What specific link did you find this behavior in?

I think ddata: is a typo, I know I can't load from that URI scheme in Chrome.

@uBlock-user
Copy link
Contributor

uBlock-user commented Jan 10, 2017

What specific link did you find this behavior in?

Try to reproduce here - http://gorillavid.in/yws7wnzv7wse

I think ddata: is a typo

I don't think so, they're trying to bypass the block now by using ddata: which is indeed invalid URI scheme.

@mapx-
Copy link

mapx- commented Jan 10, 2017

ddata on eztv.ag (chrome)
https://forums.lanik.us/viewtopic.php?p=111420#p111420

@uBlock-user
Copy link
Contributor

Using |ddata:$popup doesn't have any effect in uBO fyi

@gorhill
Copy link
Owner Author

gorhill commented Jan 10, 2017

it now tries to load popups

Does it work? The popup eventually got closed on my side with EasyList's |data:text$popup.

@uBlock-user
Copy link
Contributor

No it doesn't work, thanks to other popup blocking filters, but I came across this, so I felt like reporting. Hence the word "tries".

@gorhill
Copy link
Owner Author

gorhill commented Jan 10, 2017

Anyway, the issue here is about the -abp-properties filter, unrelated filter issues need to be reported at uAssets.

@uBlock-user
Copy link
Contributor

That domain has been swapped again and the new domain is mywatchseries.to

okiehsch added a commit to uBlockOrigin/uAssets that referenced this issue Apr 3, 2017
okiehsch added a commit to uBlockOrigin/uAssets that referenced this issue Sep 3, 2017
@gorhill gorhill closed this as completed Sep 19, 2017
okiehsch added a commit to uBlockOrigin/uAssets that referenced this issue Nov 9, 2017
okiehsch added a commit to uBlockOrigin/uAssets that referenced this issue Nov 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants