Opened on 09/23/2014 at 02:19:15 PM

Closed on 03/31/2015 at 11:11:01 AM

#1431 closed defect (worksforme)

Block element preview does not accurately demonstrate the effects of the suggested filter

Reported by: passbrains Assignee:
Priority: Unknown Milestone:
Module: Platform Keywords:
Cc: 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-67

Environment

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

How to reproduce

  1. Install ABP extension on Opera.
  1. Loads website  http://www.search.com/prefs
  2. Click on ABP and select Block element.
  3. Try to block a particular checkbox

Filter used is  www.search.com##.engine.checkbox
see image 1 and image 2

  1. Reload the page.  See image 3

Observed behaviour

After step 4, it shows blocking of 1 checkbox as shown in image 2.
After step 5, it shows blocking of all checkbox. see image 3

Expected behaviour

Step 4 and step 5 shall results in blocking same number of checkbox.

Attachments (3)

3668_1410372803_checkbox-image1.jpg (84.9 KB) - added by passbrains on 09/23/2014 at 02:19:17 PM.
3668_1410372803_checkbox-image2.jpg (78.3 KB) - added by passbrains on 09/23/2014 at 02:19:18 PM.
3668_1410372803_checkbox-image3.jpg (77.3 KB) - added by passbrains on 09/23/2014 at 02:19:20 PM.

Download all attachments as: .zip

Change History (12)

Changed on 09/23/2014 at 02:19:17 PM by passbrains

Changed on 09/23/2014 at 02:19:18 PM by passbrains

Changed on 09/23/2014 at 02:19:20 PM by passbrains

comment:1 Changed on 09/23/2014 at 02:19:25 PM by passbrains

1 - 23 Sep 2014 14:09:41 posted by Thomas Greiner
The filter is working as intended but the preview sets wrong expectations since it only checks whether the filter applies to the selected element.

comment:2 Changed on 09/23/2014 at 04:16:46 PM by greiner

  • Description modified (diff)

There are also inconsistencies between previewed and actual behavior when the "Hide placeholders" option is disabled (see #AOP-88). For this particular case we could either implement special handling or ignore it.

comment:3 follow-up: Changed on 09/24/2014 at 10:20:52 AM by sebastian

  • Cc sebastian trev added
  • Component changed from Unknown to Platform
  • Platform changed from Opera to Unknown

I already pointed out that issue in the review for #370. But Wladimir had concerns that going through the entire document looking for matching elements would go too far.

I would like to suggest again to make the "Add" button reload the page, renaming it to "Add and reload page". That would reduce complexity and fix this and similar issues (like #370 and AOP-88) in one go.

comment:4 in reply to: ↑ 3 ; follow-up: Changed on 09/24/2014 at 01:33:01 PM by trev

  • Cc sven added

Replying to sebastian:

I would like to suggest again to make the "Add" button reload the page, renaming it to "Add and reload page".

Given that a reload is likely to destroy state, that's a very clear no-go. If anything, there can be something like a checkbox to reload automatically, but I'm still not really convinced that the added UI complexity is worth it (also something to consider: the user doesn't typically know whether a reload will be free of side-effects).

Sven, any bright ideas here? How can we determine whether this is a real issue? My suspicion is that it is a minor annoyance and our users simply understand this limitation after a few times. The solutions aren't great, other than reloading we might try to identify all elements affected by a rule (similar to how Element Hiding Helper does it with its preview feature) - but that's far from trivial, still error prone (e.g. if scripts are blocked) and might produce a noticeable delay on larger pages.

comment:5 in reply to: ↑ 4 Changed on 09/24/2014 at 02:02:00 PM by sebastian

Replying to trev:

Given that a reload is likely to destroy state, that's a very clear no-go.

Fair enough, though I think as long as we make clear that a reload will occur (e.g. by calling the button "Add and reload page") it would be fine.

If anything, there can be something like a checkbox to reload automatically

That would be completely useless. If the user wants to reload the page he can already use the reload button in the browser's toolbar, after saving the filter. Anyway, making reloading the page optional, wouldn't eliminate those issues.

How can we determine whether this is a real issue? My suspicion is that it is a minor annoyance and our users simply understand this limitation after a few times.

Given this isn't the first time users and testers reported issues that resulted from differences between the immediate feedback and actual result of using the "Block Element" option I'm convinced this is an issue we should address.

The solutions aren't great, other than reloading we might try to identify all elements affected by a rule (similar to how Element Hiding Helper does it with its preview feature)

I would be fine with that approach. Also note that we already generate a list of CSS selectors matching all affected elements, in order to highlight them while the "Block Element" dialog is shown. So it wouldn't be a big deal to go a step further and hide them all once the filter is saved.

but that's far from trivial, still error prone (e.g. if scripts are blocked)

Scripts can't be selected with the mouse anyway, since they aren't visible. The only non-trivial case that currently comes to my mind is when the URL of an element affected by request blocking differs from its normalized form. But I think we can still make it work good enough.

Last edited on 09/24/2014 at 02:43:48 PM by sebastian

comment:6 Changed on 09/25/2014 at 02:01:08 PM by greiner

  • Blocking 1444 added

comment:7 Changed on 09/25/2014 at 05:21:47 PM by sven

This block element feature under google chrome....

IMO we should not invest too much time in fixing issues on this under Chrome/Opera as long as we don't decide to work on the Block element feature as a new road-map project. I think we should see reinventing the Block element feature as an own project. It doesn't help our users to have a lot of quickfixes which cause a bad user experience in the end.


comment:8 Changed on 10/21/2014 at 07:19:22 AM by sebastian

  • Component changed from Platform to User-Interface

comment:9 Changed on 03/31/2015 at 11:11:01 AM by greiner

  • Component changed from User-Interface to Platform
  • Resolution set to worksforme
  • Status changed from new to closed

This issue is no longer reproducible.

Note that since the creation of this issue, a bunch of improvements have been made to the "Block element" feature. However, I was unable to find the exact ticket which caused this issue to be resolved.

@sebastian Feel free to add that information if you can find it.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.