Opened 10 months ago

Last modified 7 weeks ago

#6387 new defect

Block element context menu does not show for divs with background image

Reported by: kzar Assignee: kzar
Priority: P2 Milestone:
Module: Platform Keywords:
Cc: Janevski, sebastian, greiner, Ross Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Environment

Chrome 64
Adblock Plus 1.13.5, no subscriptions or filters
Debian Buster

How to reproduce

  1. Browse to http://time.mk/
  2. Right click on one of the images

Observed behaviour

No "Block element" context menu item.

Expected behaviour

The option to block the element is displayed.

Notes

  • The cause is that the images are actually div elements with a background image set.
  • See the discussion here.
  • I've not looked into it but this might not actually be possible with the APIs provided, in which case we should open Firefox and Chromium issues and tag this externaldependencies.

Attachments (1)

adblockpluschrome-6387-1.zip (1.1 MB) - added by kzar 7 weeks ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 8 weeks ago by kzar

  • Owner set to kzar

I'm not 100% sure this one is possible, but I'm having a try.

comment:2 Changed 8 weeks ago by kzar

  • Cc greiner Ross added

Alright, I have a very rough proof of concept working which makes the "Block element" tool more useful in this scenario. I needed to do four things:

  1. Parse the URLs from the computed background-image style for elements.
  2. Group elements of similar size together, since elements often have multiple wrapper elements.
  3. Have the filter composer guess at which element of the group was the best candidate for blocking.
  4. Display the "Block element" context menu item for all elements.

I'm not convinced any of those things are a good idea, but I think they're worth exploring some more. Once I've tidied up my changes a bit and made a demo build, I'd really appreciate help considering the UX aspects, and help testing how well this might work in practice. I'll share the code too, but it might be too early to start reviewing the changes in detail, before we decide if any of these changes are even a good idea!

comment:3 Changed 7 weeks ago by kzar

I notice this doesn't account for elements which are absolutely positioned to occupy the same space, without being the parent element. Also, we would need to give more thought to the algorithm which decides the best element to target. It would be better to target #adpos2_1 or #ads_pane than having a filter targetting style="position:absolute;display:block;width:100%;height:100%;top:0px;padding:0px;margin:0px;" for example!

Last edited 7 weeks ago by kzar (previous) (diff)

comment:4 Changed 7 weeks ago by kzar

If you want to take a look at the experimental changes so far, I've uploaded them. Also, I've made a test build in case you want to play with it. Ideas and feedback welcome.

Changed 7 weeks ago by kzar

Note: See TracTickets for help on using tickets.