Opened on 08/13/2014 at 02:42:52 PM
Closed on 11/05/2014 at 04:57:31 PM
Last modified on 12/15/2014 at 08:10:43 AM
#1210 closed defect (fixed)
Whitelisted AFS ads are not showing up
Reported by: | fhd | Assignee: | oleksandr |
---|---|---|---|
Priority: | P2 | Milestone: | Adblock-Plus-for-Internet-Explorer-1.3 |
Module: | Adblock-Plus-for-Internet-Explorer | Keywords: | |
Cc: | oleksandr, arthur, serggzz@gmail.com | Blocked By: | #1265, #1356 |
Blocking: | Platform: | Internet Explorer | |
Ready: | yes | Confidential: | no |
Tester: | Verified working: | no | |
Review URL(s): |
Description (last modified by trev)
Environment
Adblock Plus for IE 1.2.690 with EasyList Germany and Acceptable Ads enabled.
How to reproduce
- Go to netzwelt.de
- Search for any term
Observed behaviour
The search ads are not showing up.
Expected behaviour
The search ads should show up.
Hints for testers
The change implemented here might affect other websites in unexpected ways, in particular by causing legitimate content to be blocked even though things worked correctly before. The chances that this actually happens seem to be rather low however.
Attachments (1)
Change History (19)
comment:1 Changed on 08/13/2014 at 02:44:56 PM by fhd
- Cc oleksandr added
- Resolution set to fixed
- Review URL(s) modified (diff)
- Status changed from new to closed
comment:2 Changed on 08/13/2014 at 02:45:59 PM by fhd
- Milestone set to Adblock-Plus-for-Internet-Explorer-1.2
comment:3 Changed on 08/13/2014 at 02:51:44 PM by trev
- Description modified (diff)
- Owner set to oleksandr
comment:4 Changed on 08/13/2014 at 02:56:08 PM by trev
- Resolution fixed deleted
- Status changed from closed to reopened
The original commit was backed out in https://hg.adblockplus.org/adblockplusie/rev/b241d73f074d. Please reland with proper issue reference.
comment:5 Changed on 08/13/2014 at 04:25:39 PM by sergz
The commit https://hg.adblockplus.org/adblockplusie/rev/7020c275e744 introduces another trouble.
It allows a lot of ads, which were blocked before.
Env
Acceptable ads is disabled.
How to reproduce
- Go to http://seasonvar.ru/
- Open any serial page
Observed behaviour
A lot of ads is shown, check the left side.
Expected behaviour
The ads should be blocked.
comment:6 Changed on 08/14/2014 at 03:59:39 AM by fhd
That's bad, seems like that commit Wladimir reverted did have side effects :(
Can you look into this Sergey? If this is really the cause of Acceptable Ads showing up when they have been disabled, we should definitely find a proper solution quickly - or maybe just make a 1.2 fix release that reverts this.
comment:7 Changed on 08/15/2014 at 08:50:38 AM by arthur
I cannot reproduce this. Are you using RU AdList+EasyList, Sergey?
comment:8 Changed on 08/15/2014 at 08:55:14 AM by arthur
- Cc arthur added
comment:9 Changed on 08/15/2014 at 09:02:37 AM by oleksandr
I don't see any ads there either with RU AdList + EasyList or just plain EasyList (IE11, Windows 8.1). Works as expecteed.
comment:10 Changed on 08/15/2014 at 09:12:45 AM by arthur
With EasyList only I only get the sometimes.
Changed on 08/15/2014 at 10:34:44 AM by arthur
comment:11 Changed on 08/15/2014 at 10:34:51 AM by arthur
Regarding #1210:
In the screenshot you can see how izito.de currently looks for me with ABP 1.2 on IE 11.
It looks like this hiding filter ##a[href^="http://www.google.com/aclk?"] is still being applied in the ad iframe although it shouldn't:
- The responsible whitelist for the ad iframe (@@||google.com/uds/afs$document,subdocument,domain=zoeken.nl) should make this sure via the $document option but apparently doesn't.
- Even if the $document option doesn't work, this element hiding whitelist in EasyList should show it: www.google.com#@#a[href^="http://www.google.com/aclk?"]
comment:12 Changed on 08/15/2014 at 04:10:13 PM by sergz
After discussing it in IRC some things are clear. Seems it work, I need to perform additional testing.
Despite of it I would clarify the code, I cannot find the further usage of CFilter::contentTypeAny & ~CFilter::contentTypeSubdocument; which looks like here was an idea but then something changed. Do I correctly understand that we actually should return CFilter::contentTypeAny (which is 216-1 which is a union of all other types) without subtracting any type. We might can in theory return something like CFilter::contentTypeXmlHttpRequest & CFilter::contentTypeDocument (BTW, xmlhttprequest refers to requests started by the XMLHttpRequest object, not a request which queries for xml file, but it's another issue) and feed it to the matcher via serializing to the string like "xmlhttprequest,document". For CFilter::contentTypeAny the content type string would include all types. The matcher firstly checks whether some of these types is in the white list, if so return <not-blocked, filter-entry>, otherwise check the blocking entries. As I understand now it works similar but expects only one type in the contentType.
What do you think about it, what was the aim of CFilter::contentTypeAny, why do we subtract CFilter::contentTypeSubdocument and what happened with the idea?
comment:13 Changed on 08/20/2014 at 01:54:42 PM by sergz
- Cc serggzz@gmail.com added
comment:14 Changed on 08/25/2014 at 08:01:39 AM by oleksandr
- Blocked By 1265 added
comment:15 Changed on 09/19/2014 at 01:43:37 PM by sergz
- Blocked By 1356 added
comment:16 Changed on 11/05/2014 at 04:57:31 PM by oleksandr
- Resolution set to fixed
- Status changed from reopened to closed
comment:17 Changed on 12/11/2014 at 02:10:28 PM by oleksandr
- Milestone changed from Adblock-Plus-1.2-for-Internet-Explorer to Adblock-Plus-for-Internet-Explorer-1.3
comment:18 Changed on 12/15/2014 at 08:10:43 AM by oleksandr
This issue was fixed, since issues that were blocking it are fixed now. Current issue was just a meta-issue for those. However this one had real world manifestations, and could be observed by the user, unlike the underlying issues.
This was committed without issue in 1.2: https://hg.adblockplus.org/adblockplusie/rev/7020c275e744
We should reland this referencing this issue.