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

[Safari] In Content Blocker API mode, Adguard converts rule too broadly #262

Closed
aitte2 opened this issue Jun 2, 2016 · 8 comments
Closed

Comments

@aitte2
Copy link

aitte2 commented Jun 2, 2016

I don't know if this can be fixed, or if it's a limitation of the content blocker API.

To Reproduce:

  • Use Safari
  • Enable Content Blocker API mode (there is no problem in old-API mode)
  • Subscribe to the "EasyPrivacy" list, which contains a problematic rule: ||list-manage.com/track/
  • Now you cannot visit any newsletter links sent via the popular list-manage service anymore.
  • Try to load this page: http://abraham-hicks.us4.list-manage.com/track/click?u=1c8e6d19e03449b5d28ee9f3d&id=56c46fcce1&e=88926e2c08, you will be taken to "about:blank" by Safari. You can find other test links by Googling "list-manage.com/track".

I talked to EasyList about the entry on the list, and the author defended it by saying that their HTML newsletters contain tracking-pixels to track that you have read the email (which is what the rule blocks), and by saying that the rule as-written does not block GOING to the page, only LOADING embedded resources from it, which he is of course right about.

This is a problem with the Safari Content Blocker. I wonder if you know what the issue is? Is it something about how the rule is being converted? Is it being converted to a "don't go here, don't load from here" Content Blocker rule instead of just a "don't load from here" rule, for instance?

Can you investigate it please? It's breaking a pretty big part of the web: Opening newsletter links sent by the most popular newsletter software.

@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

Yep, I am aware of this.

Not sure if mention works that way, but anyway, let's try to summon fanboy here: @ryanbr

The root cause of this issue is that Safari does not make any difference between "document" and "subdocument". Let me elaborate on this. Default behavior for Adguard and any other ad blocker is to block subdocuments (iframes), but not to block "documents" (for instance, to not mess with you clicking such links).

In the very first version converter we've tried to exclude "document" resource type while converting basic rules, but it appears that this is not very good idea, too many frames stayed unblocked.

Back then I've filed a bug report to the webkit team: https://bugs.webkit.org/show_bug.cgi?id=153559

However, it seems that they are not eager to further develop the content blocking API. At least I've seen almost no reaction on all the feature requests.

Temporary solution for this issue is to add @@||list-manage.com/track/click$document rule. We'll do it in Safari filter (which sole purpose is to fix such issues).

ameshkov added a commit to AdguardTeam/AdguardFilters that referenced this issue Jun 2, 2016
ameshkov added a commit to AdguardTeam/AdguardFilters that referenced this issue Jun 2, 2016
@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

@aitte2 could you please check this @@||list-manage.com/track/click$document rule in your user filter?

@aitte2
Copy link
Author

aitte2 commented Jun 2, 2016

@ameshkov Ah Content Blocker API limitations again. :( I wish Apple would take it seriously and improve the API, it's a super high-performance blocker but the API is so half-finished...

I tested your unbreak rule above and it works!

@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

@ameshkov Ah Content Blocker API limitations again. :( I wish Apple would take it seriously and improve the API, it's a super high-performance blocker but the API is so half-finished...

Totally.

@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

I tested your unbreak rule above and it works!

Thanks, added it to the Safari filter.

@ameshkov ameshkov closed this as completed Jun 2, 2016
@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

In addition, it would be nice if you add a comment about it to the webkit bugzilla. More people bugging them about it, greater chance one day they will do it.

Having these two issues resolved would be really great:
https://bugs.webkit.org/show_bug.cgi?id=153559
https://bugs.webkit.org/show_bug.cgi?id=152598

@aitte2
Copy link
Author

aitte2 commented Jun 2, 2016

@ameshkov Because I also care deeply about this issue, I have commented on both bugs. :) It's so close to perfect... I wish they would take it all the way to perfection.

@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2016

@aitte2 thank you!

ameshkov added a commit that referenced this issue Dec 28, 2018
…ad_notifications to master

* commit '46159fc8a9f99d3bf727f8eb84496153ae6f3ae1':
  fix
  fix viewed notifications
  fix review comments
  Add translations
  update version
  make cross color same as text color
  feature/ad_notifications
  feature/ad_notifications
  feature/ad_notifications
  move languages in the notifications
  refactoring and update icon on notification disabling
  add notifications module
  show ad notifications
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants