Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#1427 closed defect (incomplete)

Block element dialog should suggest more specific filters to reduce false positives

Reported by: passbrains Assignee:
Priority: P3 Milestone:
Module: Platform Keywords:
Cc: greiner, mapx, sebastian, trev, sven Blocked By:
Blocking: #1444 Platform: Unknown
Ready: no Confidential: no
Tester: Verified working: no
Review URL(s):

Description (last modified by greiner)

Adapted from https://www.passbrains.com/dashboard/view-ticket.php?ticket_no=AOP-56

Environment

Windows + Vista 64bit + Opera + English
ABP version Adblock Plus Version 1.8.4.1219

How to reproduce

  1. Install ABP extension on Opera.
  2. Loads a website  http://www.nokia.com/in-en/support/product/nokia-lumia-800/
  3. Scroll down the bottom of the page where you see Blue wraper with Contact support- Discussions etc.
  4. Block the wrapper mentioned in step 3.

filter displayed is  http://www.nokia.com##.wrapper
see image 1.

  1. Reload the page. observe image 2

Filter used is  www.nokia.com##.wrapper

Observed behaviour

Whole content of the website is blocked and blank page is displayed.
 

Expected behaviour

Only wrapper shall be blocked and other content of the website shall be displayed.

Attachments (2)

3668_1410091645_nokia-image1.jpg (95.2 KB) - added by passbrains 5 years ago.
3668_1410091645_nokia-image2.jpg (44.1 KB) - added by passbrains 5 years ago.

Download all attachments as: .zip

Change History (17)

Changed 5 years ago by passbrains

Changed 5 years ago by passbrains

comment:1 Changed 5 years ago by passbrains

1 - 09 Sep 2014 14:00:46 posted by Philip Hill
I cannot reproduce this. What is the filter that you have added? Mine was www.nokia.com##.w-contact_tiles and the page despite the wrapper displayed fine.

2 - 10 Sep 2014 11:43:10 posted by Timo Scherf
#4 - 09 Sep 2014 14:00:46 posted by Philip Hill
I cannot reproduce this. What is the filter that you have added? Mine was www.nokia.com##.w-contact_tiles and the page despite the wrapper displayed fine.

3 - 10 Sep 2014 17:15:34 posted by Garima Agrawal
Filter used is www.nokia.com##.wrapper

4 - 22 Sep 2014 14:08:30 posted by Thomas Greiner
I was able to reproduce it. The issue is that there are multiple elements on the page that use "wrapper" as a class. We may be able to improve the filter creation process to suggest a more specific filter (e.g. www.nokia.com###main .grid-full .w-contact_tiles .wrapper).

comment:2 Changed 5 years ago by greiner

  • Cc greiner added
  • Component changed from Unknown to Platform
  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Summary changed from On blocking the wrapper, whole website content gets blocked. to Block element dialog should suggest more specific filters to reduce false positives

A possible solution here would be to walk up the DOM tree until we find an element with an ID to construct a filter similar to the one mentioned in my previous comment which includes the intermediary elements as well.

comment:3 Changed 5 years ago by mapx

  • Cc mapx added

comment:4 follow-up: Changed 5 years ago by sebastian

  • Cc sebastian added
  • Platform changed from Opera to Unknown

Blocking all elements with the given class on the page might be as well the desired behavior in other scenarios. So improving the result for this particular case by making the filter more distinct, might just impair the result in other cases.

comment:5 in reply to: ↑ 4 Changed 5 years ago by greiner

Replying to sebastian:

Blocking all elements with the given class on the page might be as well the desired behavior in other scenarios. So improving the result for this particular case by making the filter more distinct, might just impair the result in other cases.

Obviously, we can only assume what people expect when they use the block element feature. My assumption is that they want to block a specific element. Still, if they want to make it more generic they can still modify the filter by removing parts which is at least easier than adding more to it.
We could even introduce a checkmark in the dialog labeled "block similar elements" to satisfy both needs.

comment:6 Changed 5 years ago by sebastian

  • Cc trev added

I don't have a strong opinion here. Though I'm not convinced whether this change will improve the overall experience to a degree that it justifies the added complexity, if at all.

@trev: What do you think?

comment:7 Changed 5 years ago by philll

From what I have seen and read so far, it seems that the Firefox filter creation matches user expectations on way more cases than the one of ABP for the platform module's platforms.

comment:8 Changed 5 years ago by greiner

Currently, those are the two extremes that we have: While the process in Firefox is very detailed and flexible, the one in Chrome is simple but static.

IMHO AdBlock's approach is actually quite good. They have a slider that allows you to adjust the specifity of the filter and show you the results in real-time.

comment:9 Changed 5 years ago by trev

  • Cc sven added

I guess with Firefox we mean Element Hiding Helper?

First of all: what kind of filters can we possibly generate automatically? I think it's these:

  1. ID-based filters - these are safe.
  2. Class-based filters - still good but occasionally dangerous. It might be possible to improve these by finding an element with ID attribute close to it (previous sibling or parent).
  3. Filters based on other attributes (typically the longest one) - bad. Additional qualifying parameters are typically needed.
  4. Tag-based filters - bad. These definitely need some qualifying parameters.
  5. Position-based filters - very bad, strongly discouraged.

Element Hiding Helper allows defining any of these, but only the first four can be preselected (without additional qualifying parameters, the first option that's possible will be chosen). Problem is, we don't really provide people with a set of options - it's either the default selection (which can be a very bad tag-based filter) or manual tweaking of filter parameters. That's why Element Hiding Helper is labeled "advanced functionality." At least it has preview functionality so the effects of a bad filter are typically visible immediately.

I think that we should (both in Element Hiding Helper and Adblock Plus for Chrome) give the user a selection of automatically generated filters. These filters should be as safe as possible, meaning that for any elements without an ID we should try to find another in the "neighborhood" that does - so it would be #foo > .bar or #foo + .bar rather than just .bar. We should have preview functionality so that the effects of the filter are visible immediately. And we should allow "editing" a suggestion which would go into our current advanced Element Hiding Helper UI. Oh, and that also means separating the "blocking" from the "hiding" UI in Chrome, and be it as separate tabs - suggesting to add both blocking and hiding filters was a horrible design decision IMHO.

Not really a simple change but I don't think our current Chrome UI can be made proper with simple changes.

@Sven, any comments?

comment:10 follow-up: Changed 5 years ago by sven

i just skimmed over the issue and the comments.

First of all: the issue exists because the user doesn't see the live preview (and with it the filter fail). The element picked with the Block element tool is hidden but the rest of this website is still there in spite of the filter rule is telling that all elements with class name wrapper should disappear. So if i didn't get it wrong the conclusion would be that we need a better live preview.

Second: i agree with @trev that we need a more fine granular element picking method.

greiner comment 8
IMHO AdBlock's approach is actually quite good. They have a slider that allows you to adjust the specifity of the filter and show you the results in real-time.

That's what i had in my mind, too. It's user friendly but not that accurate. I hope we can develop something better.

comment:11 in reply to: ↑ 10 Changed 5 years ago by greiner

Since we're now talking about different topics I'd like to split this into separate issues. I'll create a meta issue to keep track of those efforts:

I'd like to keep this issue about what filters we should suggest by default and it seem that we can agree on providing a more specific filter using a process as outlined by Wladimir and me.

The preview accuracy issue is already being discussed at #1431 so please continue that discussion there.

Replying to sven:

greiner comment 8
IMHO AdBlock's approach is actually quite good. They have a slider that allows you to adjust the specifity of the filter and show you the results in real-time.

That's what i had in my mind, too. It's user friendly but not that accurate. I hope we can develop something better.

Could you please create a new issue on how we could improve the block element UI?

comment:12 Changed 5 years ago by greiner

  • Blocking 1444 added

comment:13 follow-up: Changed 5 years ago by sebastian

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

I agree that there is potential to improve the way those filters are generated, and how they are previewed. But what we are talking about now, doesn't have much to do with the original bug report.

Yes, it is a symptom and shows we need to improve. However, until we decide how to improve the filter generation, this is not even a bug, since everything works as currently intended.

Also the approaches suggested by greiner and trev don't address the particular case reported above, since there is no sibling with an ID, and the closest ancestor with an ID is the same for all elements with class="wrapper" on that page.

However, feel free to work out a concept and file a change request.

comment:14 in reply to: ↑ 13 Changed 5 years ago by greiner

Replying to sebastian:

I agree that there is potential to improve the way those filters are generated, and how they are previewed. But what we are talking about now, doesn't have much to do with the original bug report.

It does, by making suggested filters more specific (see example in my first comment) we can avoid this kind of issue in which the expected behavior doesn't match the actual result.

Also the approaches suggested by greiner and trev don't address the particular case reported above, since there is no sibling with an ID, and the closest ancestor with an ID is the same for all elements with class="wrapper" on that page.

As I described we should also consider intermediary elements and not only the closest one with an ID and the selected element.

comment:15 Changed 5 years ago by sebastian

That's all fine. But it doesn't belong into this issue.

Note: See TracTickets for help on using tickets.