Opened 4 years ago

Closed 4 years ago

#3170 closed defect (fixed)

Sitekey exception rules do not apply recursively in adblockpluschrome

Reported by: kzar Assignee: greiner
Priority: P2 Milestone: Adblock-Plus-for-1.9.4-Chrome-Opera-Safari
Module: Platform Keywords: sitekey recursive
Cc: greiner, trev, sebastian Blocked By:
Blocking: Platform: Chrome
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29328928/

Description (last modified by kzar)

Environment

Adblock Plus for Chrome version 1.9.3 (stable)

How to reproduce

  1. Grab the script from https://gist.github.com/kzar/19c54d2456c0f060e9fc
  2. Run it with Python
  3. Browse to http://localhost:5000
  4. Add the two filters that are listed at the top of the page (ads and the sitekey exception rule)
  5. Refresh the page

Observed behaviour

Only the image outside of the frame is shown.

Expected behaviour

Both the image inside and the image outside of the frame should be shown. (This happens with the Firefox extension.)

Change History (11)

comment:1 Changed 4 years ago by greiner

  • Resolution set to invalid
  • Status changed from new to closed

Due to platform specifics, we look at the HTML "data-adblockkey" attribute only in Firefox and Safari while under Chromium we look at the "X-Adblock-Key" HTTP header. So as mentioned in the documentation both methods need to be implemented for it to work properly.

Therefore please check whether this issue also appears when the header is set and reopen it if that's the case.

comment:2 Changed 4 years ago by kzar

But I'm pretty sure I am returning the X-Adblock-Key header correctly already.

$ curl -I http://localhost:5000
HTTP/1.0 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 575
X-Adblock-Key: MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBANGtTstne7e8MbmDHDiMFkGbcuBgXmiVesGOG3gtYeM1EkrzVhBjGUvKXYE4GLFwqty3v5MuWWbvItUWBTYoVVsCAwEAAQ==_ACqpNMPLn02zyLJMQY98QyJB89DHPQUvWa/hW4aA82A9t3DZzUuCHXGYfF2WNnHqSyQZdjdnimLujyiGkNiAUA==
Server: Werkzeug/0.10.4 Python/2.7.9
Date: Wed, 07 Oct 2015 11:33:11 GMT

comment:3 Changed 4 years ago by greiner

  • Description modified (diff)
  • Resolution invalid deleted
  • Status changed from closed to reopened

Sorry, wasn't checking throughly enough. Indeed, the image inside the frame is not shown but that's not because it was blocked but because it was hidden. Presumably, this was done by element collapsing.

Note that I added the requirement for a blocking filter to the reproduction steps and removed EasyList which allows us to make sure that this behavior is independent of element hiding.

comment:4 Changed 4 years ago by kzar

  • Description modified (diff)

comment:5 Changed 4 years ago by sebastian

  • Priority changed from Unknown to P2
  • Ready set

Reproduced with Adblock Plus 1.9.3. This seems to be a regression, as it works correctly with 1.8.12. Firefox is fine as well. I'm going to do further bisecting.

Last edited 4 years ago by sebastian (previous) (diff)

comment:6 Changed 4 years ago by sebastian

The regression was introduced by a400e15525fc. However, I don't understand yet why that change is causing this issue.

comment:7 Changed 4 years ago by greiner

From what I see, the problem is that the sitekey is not passed to the defaultMatcher.matchesAny() call in background.js.

@sebastian Not sure either why it worked before but maybe element collapsing simply wasn't functioning properly. Let me know if you insist on investigating that aspect further because otherwise I'd start the review based on the reason I mentioned above.

comment:8 Changed 4 years ago by sebastian

Yeah, I already gave up on anaylzing that change, and started to debug why this frame isn't considered whitelisted. Though I didn't figured anything out there either yet. If you want to investigate that issue fine with me.

Version 0, edited 4 years ago by sebastian (next)

comment:9 Changed 4 years ago by greiner

  • Owner set to greiner

comment:10 Changed 4 years ago by greiner

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

comment:11 Changed 4 years ago by greiner

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.