Opened on 06/03/2014 at 03:41:16 PM
Closed on 10/05/2015 at 12:07:18 PM
Last modified on 10/08/2015 at 01:58:50 PM
#616 closed change (fixed)
Add $generichide filter option that can be used to disable all generic element hiding filters for a website
Reported by: | Crits | Assignee: | kzar |
---|---|---|---|
Priority: | P2 | Milestone: | Adblock-Plus-for-1.9.4-Chrome-Opera-Safari |
Module: | Core | Keywords: | 2015q3 |
Cc: | famlam, mapx, arthur, trev, manvel, greiner, sergz, ben | Blocked By: | #2738 |
Blocking: | Platform: | Unknown | |
Ready: | yes | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
https://codereview.adblockplus.org/5840485868371968/ |
Description (last modified by kzar)
Background
We need a way to disable all generic element hiding rules for a given website. At the moment we can only disable each generic filter individually which is time consuming and verbose!
- See this ABP forum topic for more background.
- Also see ticket #647 which is very similar and interrelated but for generic request blocking rules instead of generic hiding ones.
What to change
Add the new $generichide filter option to the lib/filter... code in the adblockplus repository.
It should act similarly to the existing $elemhide option but instead of matching all element hiding rules it should only match the generic ones.
When a $generichide filter is applied to a page it should also recursively apply to all child frames.
Generic filter rules count as rules that:
- Do not have a domain specified. "Hide this element on all domains"
- Have only domain exceptions specified. "Hide this element on all domains except example.com"
Example generic filters:
- ~example.com##.ad
We also need to update the adblockplus dependency for the adblockpluschrome repository from bdd5cd8bca3a to 893426c6a6ab. The following unrelated changes to adblockplus have happened since then:
- d9aea02b4c79 - "Noissue - Fixed typo in Indonesian translation for viewList.label"
- a463f8a01e5f - "Issue 3086 - Add mapping for Adblock Browser logo on the first run page"
- 2d8be2fd147b - "Releasing Adblock Plus 2.6.11"
- 8a335396ba99 - "Added tag 2.6.11 for changeset 2d8be2fd147b"
- 098d48f73764 - "Noissue - reverted unintentionally checked in debug changes"
- 29d524332056 - "Releasing Adblock Plus 2.6.11"
- 2c481ae80add - "Added tag 2.6.11 for changeset 29d524332056"
- The first simply fixes a mistake in an Indonesian translation, this should not matter to us.
- The second is simply adding a mapping for an ABB related graphic, again not relevant to us.
- Finally the last five focus on releasing version 2.6.11 and should not cause us any problems either.
Finally we need to update the adblockplustests dependency for the adblockpluschrome repository from ae6c7a2cc723 to 449058480108. The following unrelated changes to adblockplustests have happened since then:
- afcb18d3311b - "Issue 284 - Unify initial delay for all downloads"
- 04616196f39b - "Noissue - Fixed syntax error (invalid left-hand expression)"
- 7e924f798246 - "Noissue - Marked compatible with Firefox 43 & Co."
- 63747b8afb94 - "Issue 2953 - Cleanup on unload is broken"
- c1b619da264f - "Noissue - Fix SVG unit tests"
- The first, second and fifth changes were to tests that aren't included by the Chrome extension test suite. (synchroniser, icon_position and policy respectively)
- The third change was to the metadata.gecko file which again is not used by the Chrome tests.
- The fourth change was to fix a problem with cleanup after some of the asynchronous unit tests. I don't think the issue showed up in any of the Chrome tests, but if anything adding this change should make things run more reliably.
I have also tested that these changes and no additional tests are failing as a result. (9 CSS escaping tests are currently failing but that is unrelated)
Attachments (0)
Change History (49)
comment:1 Changed on 06/03/2014 at 05:33:38 PM by mapx
- Cc mapx added
comment:2 follow-up: ↓ 3 Changed on 06/04/2014 at 08:30:40 AM by arthur
- Cc arthur added
- Priority changed from Unknown to P3
- Ready set
comment:3 in reply to: ↑ 2 Changed on 06/05/2014 at 04:32:11 PM by Crits
Replying to arthur:
Thanks for creating this issue. I will also create one for generic blocking rules as well.
That would be sooooo great!
comment:4 Changed on 06/06/2014 at 08:09:34 AM by arthur
- Priority changed from P3 to P2
comment:5 Changed on 06/07/2014 at 09:59:25 AM by Crits
- Component changed from Unknown to Core
comment:6 Changed on 07/07/2014 at 12:06:05 PM by trev
- Cc trev added
- Platform set to Unknown
comment:7 Changed on 07/22/2014 at 04:09:53 PM by saroyanm
- Cc manvel@adblockplus.org added
comment:8 follow-up: ↓ 10 Changed on 09/09/2014 at 08:21:04 AM by greiner
Rather than introducing a new option, what about reusing elemhide for that? In such a case <domain>$elemhide would apply to all element hiding filters while <domain>$elemhide=~<domain> would only apply to filters that are not specific to that domain. Would that be sufficient?
However, there might be backwards-compatibility problems so <domain>$elemhide=~<domain> would probably be equal to <domain>$elemhide in older versions that don't support this syntax. Although implementing $generichide would probably have similar issues in this regard.
@trev: Regarding the technical feasibility of this feature, I wonder whether this would be blocked on #521.
comment:9 Changed on 09/09/2014 at 08:58:34 PM by trev
Although implementing $generichide would probably have similar issues in this regard.
Currently, the typical approach is writing rules like foo$generichide,image,~image to avoid this. For older versions (ones that don't know generichide) this is a rule not matching any types. For newer versions it is equivalent to foo$generichide.
@trev: Regarding the technical feasibility of this feature, I wonder whether this would be blocked on #521.
No, #521 doesn't affect this any more than it affects usual element hiding exceptions for example.
comment:10 in reply to: ↑ 8 Changed on 09/09/2014 at 10:22:54 PM by famlam
Rather than introducing a new option, what about reusing elemhide for that? In such a case <domain>$elemhide would apply to all element hiding filters while <domain>$elemhide=~<domain> would only apply to filters that are not specific to that domain. Would that be sufficient?
Although I would prefer the re-use of options, I'd rather suggest the use of boolean options. $elemhide=... would give a lot of possibilities that all have to be taken care of, e.g. <domain>$elemhide=~<domain2> or <domain>$elemhide=<domain> and all kind of combinations such as <domain.com>$elemhide=~sub.domain.com|domain.com|~com if we want to exclude certain rules only. Besides, it's slightly confusing as it has a completely different interpretation than the similar-syntax $domain= option.
I can think of a couple of those boolean options. The simplest is ~elemhide, which only purpose currently could be that ~elemhide is shorthand for ~image,image (e.g., match all resource types), although I cannot recall any occurrence of that so far.
Another possibility is ~third-party: adding this would get rid of all domain-specific (=first party) rules, which would be more easily to understand, even though the meaning is slightly different than ~third-party for regular rules (as hiding rules are of course by definition first-party, except for in subframes)
Another option is to completely replace the present elemhide with the newly suggested elemhide. The need for the current behavior is rare: you don't want to whitelist specific rules if you can as well remove those (or, in case of an external filter list that doesn't want to cooperate, you could eventually whitelist them with #@# rules). Requiring the current elemhide behavior on third-party subframes is even more rare, I cannot think of a realistic use case for that. Furthermore, elemhide is only known to list authors and the few users who have read the time to learn the syntax. It's not an option that can be added from within the extension (e.g. EHH or the block-a-resource dialog).
My first and second suggestion can also be ported to blocking rules (e.g. ~document, issue #647). The third suggestion would be specific to hiding rules.
However, there might be backwards-compatibility problems so <domain>$elemhide=~<domain> would probably be equal to <domain>$elemhide in older versions that don't support this syntax. Although implementing $generichide would probably have similar issues in this regard.
Wasn't this dealt with in a way that filters with unknown options were completely discarded, such that we don't even need the $image,~image syntax anymore? At least in EL we do no longer use $image,~image for $popup rules...
comment:11 Changed on 09/16/2014 at 08:57:51 PM by trev
- Priority changed from P2 to P3
comment:12 Changed on 10/06/2014 at 11:46:07 AM by fhd
- Keywords 2014q4 added
- Priority changed from P3 to P2
comment:13 Changed on 10/06/2014 at 02:23:33 PM by greiner
- Cc greiner added
comment:14 follow-up: ↓ 16 Changed on 10/06/2014 at 03:48:23 PM by greiner
Replying to famlam:
Besides, it's slightly confusing as it has a completely different interpretation than the similar-syntax $domain= option.
Isn't that the general problem with the $elemhide filter option? While other options are specifying when a filter should be applied, $elemhide specifies what should be applied. On top of that it defines element hiding behavior within a blocking filter. Adding another element hiding-specific option to blocking filters would make it even more confusing than it already is.
Therefore I'm leaning towards either extending $elemhide in some way or introducing filter options to element hiding filters. While I could also imagine a completely new type of filter for domain-specific options (e.g. $document, $elemhide) rather than resource-specific ones (e.g. $image, $collapse), it appears out of scope for this issue.
My first and second suggestion can also be ported to blocking rules (e.g. ~document, issue #647).
While this might be the way of least resistance in regard to implementation, I can imagine this to make it even more difficult to comprehend what the keyword elemhide actually means (e.g. @@||example.com$elemhide disables all element hiding filters; @@||example.com$~elemhide disables only generic element hiding filters). The same issue applies to $~document.
comment:15 Changed on 10/06/2014 at 05:37:03 PM by famlam
And what about completely replacing the current $elemhide behavior? As explained in my previous post, I don't think there is much reason to have the present behavior over the suggested behavior. (And it would hardly, if any at all, cause compatibility issues with currently existing filters). Furthermore, you would prevent that all filter lists have to update their filters to the new variant, as well as keeping an old rule for older ABP versions (until most people have updated).
But if you really want to extend $elemhide with options ($elemhide=X), I wouldn't choose the domain as it is rare that $elemhide occurs in a filter not containing a domain already (e.g. @@/ads/*$elemhide would always be way too generic, compared to @@||DOMAIN/ads/$elemhide or @@/ads/*$elemhide,domain=DOMAIN, and @@||site.com^$elemhide=~anyOtherSite.com would be meaningless). That way, you would only add duplicate information to a filter. Better (IMO) might in that case be something like $elemhide=generic (or some other fixed-value-option). I guess code-technically it would also be beneficial for speed, as you have to match the full filter anyway, but have no additional comparisons for the domain of $elemhide.
comment:16 in reply to: ↑ 14 ; follow-up: ↓ 18 Changed on 10/06/2014 at 07:28:31 PM by trev
Replying to greiner:
Isn't that the general problem with the $elemhide filter option? While other options are specifying when a filter should be applied, $elemhide specifies what should be applied.
We actually have both kinds of options. We have those making the filter more specific (type options, $domain and similar) and those activating non-default behavior ($document, $collapse).
On top of that it defines element hiding behavior within a blocking filter.
Since it applies to URLs rather than selectors, it actually makes sense.
Therefore I'm leaning towards either extending $elemhide in some way
That would overload the meaning here and make things extremely confusing.
or introducing filter options to element hiding filters.
That would make no sense whatsoever - as I said already, this isn't about selectors.
Actually, both this and #647 could be implemented by means of a new $generic option: @@||example.com^$generic (disable generic filters of any kind on example.com domain).
Replying to famlam:
And what about completely replacing the current $elemhide behavior? As explained in my previous post, I don't think there is much reason to have the present behavior over the suggested behavior. (And it would hardly, if any at all, cause compatibility issues with currently existing filters).
This is indeed an option. It has the advantage that we wouldn't need to support three different whitelisting schemes for element hiding rules ($document, $elemhide and $generic), and conventional filter lists indeed rarely intend to override element hiding rules that have been targeted specifically at a particular domain. The only place where this would become problematic is the Acceptable Ads list, it would have to use specific exceptions for some rules (not sure how many) if we changed $elemhide behavior. My impression is that this would make maintaining that list harder.
comment:17 follow-up: ↓ 19 Changed on 10/06/2014 at 08:55:47 PM by famlam
Actually, both this and #647 could be implemented by means of a new $generic option: >@@||example.com^$generic (disable generic filters of any kind on example.com domain).
The problem I see with $generic is that it would also more-or-less kill EasyPrivacy and other non-ads-lists on websites where basically only generic element hiding rules have to be disabled.
Many cases where we have to add $elemhide rules due to Anti-Adblock measurements, we only need a few blocking whitelists, and often enough those blocking whitelists are for resources of which the server is blocked (e.g., rules starting with ||, which according to the discussion-in-which-I-do-not-participate in #647 would be non-generic). It's very rare to find Anti-Anti-Tracking or Anti-Anti-Social measurements on websites though, so there is no need to whitelist them altogether.
The only place where this would become problematic is the Acceptable Ads list, it would have to use specific exceptions for some rules (not sure how many) if we changed $elemhide behavior.
Just to sketch the impact: according to my redundancy checker there are ~125 specific hiding rules (in EL+ELD) being made redundant by $elemhide rules in the Acceptable Ads list, which means ~125 #@# rules have to be added to the AA list (+ ones that are triggered by specific rules in other regional lists) if this method is chosen. (Furthermore, off topic, it would allow using custom hiding rules on those AA domains, which is currently impossible).
comment:18 in reply to: ↑ 16 Changed on 10/06/2014 at 10:24:23 PM by famlam
comment:19 in reply to: ↑ 17 Changed on 10/07/2014 at 06:21:32 AM by trev
Replying to famlam:
Just to sketch the impact: according to my redundancy checker there are ~125 specific hiding rules (in EL+ELD) being made redundant by $elemhide rules in the Acceptable Ads list, which means ~125 #@# rules have to be added to the AA list (+ ones that are triggered by specific rules in other regional lists) if this method is chosen.
The main problem here isn't the one-time effort of adding these rules, it's keeping them in sync with EasyList & Co. that's problematic.
comment:20 Changed on 12/09/2014 at 03:45:14 PM by sergz
- Cc sergz added
comment:21 follow-up: ↓ 22 Changed on 12/09/2014 at 04:32:27 PM by Lain_13
Hi, I'm not sure it have to be a part of this issue, so feel free to create a new one but I think elemhide have to disable collecting hit statistics in FireFox for specific site since it reveals presence of active AdBlock through property -moz-binding (example here: iphones.ru search for ".getPropertyValue('-moz-binding').indexOf('about:')" in the page code). Maybe it have to be a separate option but it will be a fix specific only for Firefox then.
comment:22 in reply to: ↑ 21 Changed on 12/10/2014 at 11:32:10 AM by greiner
Replying to Lain_13:
Hi, I'm not sure it have to be a part of this issue, so feel free to create a new one but I think elemhide have to disable collecting hit statistics in FireFox for specific site since it reveals presence of active AdBlock through property -moz-binding (example here: iphones.ru search for ".getPropertyValue('-moz-binding').indexOf('about:')" in the page code). Maybe it have to be a separate option but it will be a fix specific only for Firefox then.
Sorry but I couldn't find that in the page code. Please create a new issue if you can provide more details about it. Generally, example code and pages would be quite helpful.
comment:23 Changed on 12/23/2014 at 01:51:24 PM by kzar
- Owner set to kzar
comment:24 Changed on 12/23/2014 at 02:20:00 PM by kzar
I'm not clear how this ticket differs from #647? (Maybe one of them should be closed?)
Also there are lots of different approaches suggested in the comments of this ticket, could you check the description is accurate and maybe update it @palant / @greiner? Just so it's clear what exactly we're doing before I start. So far there are a couple of quotes in the description, describing things that would be nice but nothing concrete that says exactly what we're going to do.
comment:25 Changed on 12/23/2014 at 02:20:19 PM by kzar
- Ready unset
comment:26 Changed on 01/06/2015 at 03:14:25 PM by arthur
@kzar
#647 is for blocking rules, this one is for hiding rules.
comment:27 Changed on 01/15/2015 at 04:21:09 PM by greiner
- Description modified (diff)
I reflected the common consens in the issue description.
comment:28 Changed on 01/16/2015 at 05:15:30 PM by kzar
- Description modified (diff)
- Summary changed from Disable generic hiding rules only + lessen the effect of $elemhide to Add $generichide filter option that can be used to disable all generic filters for a website
comment:29 Changed on 01/16/2015 at 05:16:56 PM by kzar
- Description modified (diff)
- Summary changed from Add $generichide filter option that can be used to disable all generic filters for a website to Add $generichide filter option that can be used to disable all generic element hiding filters for a website
comment:30 Changed on 01/16/2015 at 05:17:08 PM by kzar
- Ready set
comment:31 Changed on 01/19/2015 at 11:22:06 AM by kzar
- Description modified (diff)
comment:32 Changed on 02/13/2015 at 02:20:00 PM by kzar
- Description modified (diff)
comment:33 Changed on 03/06/2015 at 09:53:36 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:34 Changed on 03/11/2015 at 05:11:48 PM by kzar
- Review URL(s) modified (diff)
comment:35 Changed on 03/19/2015 at 11:03:48 AM by kzar
- Review URL(s) modified (diff)
comment:36 Changed on 03/19/2015 at 11:06:18 AM by kzar
Quick update: I've implemented $generichide + $genericblock in Firefox + Chrome and added some tests. All changes are now just awaiting review.
comment:37 Changed on 06/30/2015 at 11:35:25 AM by trev
- Blocked By 2738 added
comment:38 Changed on 07/30/2015 at 05:41:28 PM by fhd
- Keywords 2015q3 added; 2014q4 removed
- Tester set to Unknown
comment:39 Changed on 08/05/2015 at 10:43:45 AM by arthur
- Cc manvel ben added; manvel@adblockplus.org removed
comment:40 Changed on 09/05/2015 at 03:44:05 PM by kzar
- Review URL(s) modified (diff)
comment:41 Changed on 09/28/2015 at 12:39:24 PM by kzar
comment:42 Changed on 09/28/2015 at 12:47:08 PM by kzar
comment:43 Changed on 09/28/2015 at 01:17:57 PM by kzar
- Blocked By 3080 added
comment:44 Changed on 09/28/2015 at 03:27:47 PM by kzar
- Blocked By 3080 removed
comment:45 Changed on 09/28/2015 at 04:44:26 PM by kzar
- Description modified (diff)
comment:46 Changed on 10/02/2015 at 04:05:21 PM by kzar
comment:47 Changed on 10/05/2015 at 09:47:39 AM by kzar
- Description modified (diff)
comment:48 Changed on 10/05/2015 at 12:07:18 PM by kzar
- Resolution set to fixed
- Status changed from reviewing to closed
comment:49 Changed on 10/08/2015 at 01:58:50 PM by kzar
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
Thanks for creating this issue. I will also create one for generic blocking rules as well.