Opened 2 years ago

Closed 2 months ago

#5665 closed change (rejected)

Combine element hiding rules with the same selector

Reported by: Lain_13 Assignee:
Priority: P3 Milestone:
Module: Core Keywords: closed-in-favor-of-gitlab
Cc: kzar, greiner, sebastian, sergz, arthur, mjethani, hfiguiere Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by kzar)

Background

Currently there is no way to disable generic hiding filter without actually whitelisting object on specific domain. However, this is not the best idea when you support multiple filter lists of different granularity and want to whitelist something in one (like self-ads on the side), but keep it hidden in another (like the same self-ads in annoyance filters).
Also, uBO already implemented such behavior.

What to change

In case when there are multiple versions of the same generic filter with exceptions and without them drop completely generic one (without any exceptions) and combine the rest into a single filter with all the exceptions.

Site-specific hiding filters should still have precedence over such exceptions.

Test

  1. Add filters ###logo and ~adblockplus.org###logo
  2. Refresh this page.
  3. Add filter adblockplus.org###logo
  4. Refresh this page again.

Observed behaviour

ABP logo in the top-left corner is hidden on both step 2 and step 4.

Expected behaviour

ABP logo in the top-left corner is visible on step 2, but hidden on step 4.

Notes

  • Ensure that element hiding is not slowed down, e.g. getSelectorsForDomain.
  • Ensure that the developer tools panel still lists element hiding matches properly.

Change History (18)

comment:1 Changed 23 months ago by sergz

  • Cc kzar greiner sebastian sergz added
  • Component changed from Unknown to Core

comment:2 Changed 23 months ago by sergz

  • Cc arthur added

comment:3 Changed 23 months ago by kzar

  • Cc mjethani hfiguiere added
  • Description modified (diff)

I'm not sure how we'd implement this in practice, especially since we spent quite some time optimising getSelectorsForDomain in lib/elemHide.js. That said it sounds like a sensible enough suggestion on the surface on it.

Would the suggested behaviour be desirable arthur?

comment:4 Changed 23 months ago by kzar

  • Summary changed from Establish precedence of generic filters with exceptions over same filters without them to Combine element hiding rules with the same selector

comment:5 Changed 22 months ago by arthur

Makes sense to me and sounds useful. Lain_13, do you know about some cases where uBO is using it?

comment:6 Changed 22 months ago by Lain_13

As far as I know uBO doesn't use it own filter list, but that's how it process filters from filter lists from third parties like EasyList or RU AdList and other supplementary lists. This measure allows it to avoid reintroducing false positives on sites where they were already solved when multiple filter lists from different authors are in use. It also leaves ability to hide these objects for end users since domain-specific hiding filters still take precedence.

comment:7 Changed 22 months ago by kzar

  • Priority changed from Unknown to P3
  • Ready set

I'm not sure how this would be implemented in practice, but it would be nice to have. Let's see if someone™ can figure it out.

comment:8 Changed 22 months ago by kzar

  • Description modified (diff)

comment:9 Changed 17 months ago by mjethani

In the expected behavior this issue says "ABP logo in the top-left corner is visible on step 2, but hidden on step 4.". Why should it be visible on step 2? ###logo and ~adblockplus.org###logo are different filters, the first one should still apply, right?

comment:10 Changed 17 months ago by Lain_13

That's the issue. ABP treat these as two separate filters even though they are one and the same global filter. It's just one copy of it happen to have an exception to it. Take any other domain and both will apply since essentially both are global and target exactly the same object.

Example:
Let there be two filter lists, each have the same global filter in it, but with different lists of exceptions or without them entirely. Right now one of them will apply and cause problem unless there is explicit whitelist filter in one of the lists or specific domain is in the list of exceptions of both. Explicit exception makes object impossible to hide by end users and inlined exceptions rarely are the same in different filter lists.

Let's take for example:
Filterlist A:
~example.com,~someplace.org###logo
Filterlist B:
~adblockplus.org,~someplace.org###logo

Technically both are global, but each have separate list of exceptions.

Right now first one will apply here because author of filterlist A doesn't know his filter cause a problem on this particular domain. He supports list for entirely different region and nobody ever reported to him about this issue. On top of that author of filterlist B don't know about issue on example.com and his filter will cause it once again even though author of filterlist A did knew and fixed it in his list with exception to a global filter. Apparently both knew about the issue on someplace.org, so both made an exception, but that is only 1 domain out of 3 affected.

While global filter can safely supersede domain-specific hiding filters because object will be hidden in either cases they absolutely must not supersede domain-specific exceptions, because they imply that global filter must not apply on that domain. uBO handles this case and combines exceptions from multiple copies of the same global filter which reduces chance of false-positives when multiple filter-lists are installed while ABP treats identical copies of global filters with different exceptions as separate filters and effectively increase number of false-positives by doing that.

It's true that installing multiple filterlists does increase number of false-positives both in uBO and ABP, but uBO tries to mitigate the most obvious cases like this one.

Last edited 17 months ago by Lain_13 (previous) (diff)

comment:11 Changed 17 months ago by greiner

Note that I've pointed out this inconsistency in this discussion that I had with Wladimir in a code review for optimizing ElemHide.getSelectorsForDomain(). There he argued that

If you add a filter like ###foo you expect it to work. Now suddenly it depends on other filters with the same selector, probably in other filter lists.

whereas my stance was

~example.com###foo is not an exception rule but it's still an explicit intent to not apply a selector on example.com.

Unfortunately, we didn't come to a real agreement there but I hope it helps with providing some context for this discussion.

comment:12 Changed 17 months ago by Lain_13

Um... I think I disagree with Wladimir on this since I fail to see the difference between explicit whitelist and implied in case of:

If you add a filter like ###foo you expect it to work.

Since in case of explicit exception it won't work no matter do you know about that exception or not. Even if you _expect_ it to work will it actually work or not depends on a presence of explicit exceptions in other lists, so it depends on other lists in both cases. Of course it would be nice to see somewhere why exactly it didn't work as with explicit exceptions. That's the only reason why such filters might be confusing from my point of view. Otherwise there is no difference between ~example.com###foo and example.com#@##foo beside the fact that second one IS forced while first one right now ignored in ABP if there is ###foo. And unlike with explicit exceptions there always will be an option to add example.com###foo and it will work. That is actually expected behaviour even with explicit whitelist if example.com###foo is present if user's local filters. ABP doesn't differentiate filters by their source and doesn't prioritize user's local filters over those present in a list and that also is wrong. While author of filter list may know about such technical details when end user adds a filter he actually expect it to work.

BTW, I also disagree with this his statement as well:

And if you implement a minimal change to the selector this behavior will suddenly change.

Since behavior will suddenly change with explicit whitelist as well. You slightly change hiding rule and suddenly explicit exception doesn't apply to it since selectors are different even if they target exactly the same set of objects.

This is actually a problem on it's own since when you have filter example.com###foo in a list, but want to whitelist example.com#@###bar > #foo in your own list or local filters such exception won't work because even though they both target intersecting sets of objects whitelist won't apply because the selector itself is different. And with current ABP you can't even disable existing rule. This behaviour is highly inconsistent with blocking filters where you can block resources with ||example.com/ads/, but then explicitly allow @@||example.com/ads/cats/. I do understand why it's like this and unlikely to change in the near future, but that doesn't resolve an issue.

And as I told before explicit exceptions are bad because you enforce your choice with these unless end user know how it works and use a trick like example.com##* > #foo or example.com###foo:not(#bar). That will suddenly work while simple example.com###foo won't. Does it look like an obvious thing to do?

In case of ads explicit whitelist is more or less acceptable tactic since it either an ad or false positive, but nowadays ABP also hides social buttons and other annoying elements and even advertises this function to the end users. However, these buttons could be controversial since someone may want to see some of these at least in login forms and some may want to hide even such buttons. However, explicit whitelist leaves users without a simple solution and they have to know about tricks like I described above to override these.

Last edited 17 months ago by Lain_13 (previous) (diff)

comment:13 Changed 15 months ago by mjethani

Thanks for the write-up, @Lain_13. You make some valid points. I am not fully convinced that the current behavior should be changed, but I'll have to think about it.

What about the following two filters from different lists:

example.com##.foo
example.com,~www.example.com##.foo

Does this mean that the first filter should not apply on www.example.com because of the second filter?

What about this pair:

example.com##.foo
~www.example.com##.foo

Should the selector .foo be injected on www.example.com now?

These are questions we need to answer before we decide to change the behavior.

comment:14 Changed 15 months ago by Lain_13

Does this mean that the first filter should not apply on www.example.com because of the second filter?

Normally lists shouldn't have many intersecting domain-bound filters, but yes exception in the second filter should override first one since it have higher specificity (applies to specific sub-domain) and also it seems like author of the second filter know something about that. Usually it means that they should contact each other and preferably push that domain to EasyList entirely.

What about this pair:
Should the selector .foo be injected on www.example.com now?

Yes.

Unlisted case:

www.example.com##.foo
example.com,~www.example.com##.foo

In this case first filter should override exception in the second one. Usually this means that second filer came from supplementary list and author disagrees with exception in the primary list.

Also, it would be nice if ABP would show implied exceptions in the log in the similar way as explicit, so it will be obvious why filter didn't work.

comment:15 Changed 15 months ago by sebastian

I vote for rejecting this issue.

Filters like ~adblockplus.org###logo aren't exception rules, they merely restrict the domains this filter is applied on, but naturally there can still be other filters (for the same selector) with less restrictions that might match regardless.

I think the semantics suggested here are less intuitive than what is currently implemented (i.e. make filters harder to understand). Not to mention that this change will break behavior in existing filter lists.

Also it seems inconsistent with how filters work in general, e.g. if there is ||example.com^ and ||example.com^$script all requests to example.com, not only scripts, are going to be blocked.

However, in cases where you explicitly want this behavior, you can already get it using an exception rule:

###logo
adblockplus.org#@##logo

So implicitly treating domain restrictions the same way (as opposed to having filter list authors choose between domain restrictions and exception rules) makes the filter syntax less flexible.

Last edited 15 months ago by sebastian (previous) (diff)

comment:16 Changed 15 months ago by Lain_13

Once again, primary use for this change is to avoid false positives since right now:

~some.domin###logo
~another.domain###logo

Which came from two different subscriptions will hide #logo on both. To avoid confusion this logic could be applied to global filters only and domain-specific rules will always prevail. In such case answer to both mjethani's questions is no.

Last edited 15 months ago by Lain_13 (previous) (diff)

comment:17 Changed 15 months ago by sebastian

I don't see anything confusing about this behavior, but again if that is not the behavior you want, just use an exception rule.

On the other hand, there might be scenarios where this behavior is desired, for example if one filter list has a broader scope (e.g. blocking all annoyances), it would be expected to overrule domain restrictions from another filter list with a more specific scope.

comment:18 Changed 2 months ago by sebastian

  • Keywords closed-in-favor-of-gitlab added
  • Resolution set to rejected
  • Status changed from new to closed

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the new issue tracker.

Note: See TracTickets for help on using tickets.