Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#4210 closed defect (fixed)

Element collapsing uses parent frame for dynamically created frames in Chrome

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

https://codereview.adblockplus.org/29349744/

Description (last modified by kzar)

Environment

How to reproduce

  1. Add this filter: @@serve.html$subdocument,document,domain=kzar.co.uk
  2. Browse to https://static.kzar.co.uk/injected-iframe-test/index2.html

Observed behaviour

The first image is collapsed after it loads. Sometimes it is displayed for a moment before that happens.

Expected behaviour

The first image should not be collapsed as the whitelisting rule which stopped it from being blocked.

Notes

  • If you view the child frame's page directly the image is no longer collapsed.
  • Both images are displayed correctly with the Firefox extension and even with Safari 9.
  • When the image is wrongly collapsed it is not listed in the Adblock Plus devtools panel.

Change History (9)

comment:1 Changed 3 years ago by kzar

  • Description modified (diff)

comment:2 Changed 3 years ago by kzar

So we're sending the filter.collapse message from the parent frame[1], which is why we're getting the wrong result. We do know the correct URL though, it's contentDocument.location.href, so there's probably a way we can work around this. Continuing to play with the code!

[1] - https://github.com/adblockplus/adblockpluschrome/blob/master/include.preload.js#L553-L560

comment:3 Changed 3 years ago by kzar

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

comment:4 Changed 3 years ago by kzar

  • Summary changed from Whitelisted blocked image is still collapsed when inside of frame to Element collapsing uses parent frame for dynamically created frames in Chrome

comment:5 Changed 3 years ago by sebastian

  • Owner changed from kzar to sebastian
  • Review URL(s) modified (diff)

comment:6 Changed 3 years ago by abpbot

comment:7 Changed 3 years ago by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:8 Changed 3 years ago by sebastian

For reference, I partly removed the workaround introduced with #581. The code removed caused overeager element collapsing in the case demonstrated in this issue. Removing that code didn't seem to bring back the placeholders on chip.de as described in #581, that however could be as well because they changed their website (or the responsible Chrome bug got fixed meanwhile). Either way, that workaround was quite a hack, and seeing some placeholder in edge cases is better than blocking too much.

comment:9 Changed 3 years ago by scheer

  • Tester changed from Unknown to Scheer
  • Verified working set
  • Both images are not collapsed and are shown correctly when the filter is applied.

ABP 1.12.1.1642
Chrome 38
Chrome 51
Opera 30
Opera 38
Safari 9.1.2

Note: See TracTickets for help on using tickets.