#6797 closed change (fixed)
Require active domains in snippet filters
Reported by: | mjethani | Assignee: | mjethani |
---|---|---|---|
Priority: | P2 | Milestone: | |
Module: | Core | Keywords: | circumvention |
Cc: | jsonesen, kzar, wspee, greiner, sebastian | Blocked By: | |
Blocking: | #6538, #6846 | Platform: | Unknown / Cross platform |
Ready: | yes | Confidential: | no |
Tester: | Ross | Verified working: | yes |
Review URL(s): |
Description (last modified by mjethani)
Background
Similar to element hiding emulation filters, snippet filters should also be required to have active domains. This is for security and performance reasons.
What to change
Add a check in ContentFilter.fromText: If there are no active domains in the snippet filter, return an InvalidFilter instance with the appropriate reason.
Integration notes
The change for this adds a new "reason" for the invalid filter called filter_snippet_nodomain. It needs a change in the UI. The text should be similar to the text for filter_elemhideemulation_nodomain.
Hints for testers
Test the following types of generic filters:
- Given three snippet filters, #$#log foo-1, example.com#$#log foo-2, and ~www.example.com#$#log foo-3, only foo-2 should be printed in the DevTools console when visiting example.com.
- Given three element hiding emulation filters, #?#.foo-1, example.com#?#.foo-2, and ~www.example.com#?#.foo-3, only example.com#?#.foo-2 should be applied on example.com (see the Adblock Plus DevTools panel for this).
- Given three element hiding filters, ##.foo-1, example.com##.foo-2, and ~www.example.com##.foo-3, all three filters should be applied on example.com.
Change History (13)
comment:1 Changed 17 months ago by mjethani
- Description modified (diff)
comment:2 Changed 16 months ago by mjethani
comment:3 Changed 16 months ago by mjethani
- Owner set to mjethani
comment:4 Changed 16 months ago by mjethani
- Description modified (diff)
comment:5 Changed 16 months ago by greiner
- Cc wspee greiner added
comment:6 follow-up: ↓ 7 Changed 16 months ago by kzar
- Cc sebastian added
Well, this new string isn't located anywhere yet :p. Couldn't we just add it to adblockplusui, then migrate the other similar ones over in the future? (Yea, it would be kind of inconsistent.)
Anyway, I'll mention this to Sebastian, see if we can agree on something re #6633.
comment:7 in reply to: ↑ 6 Changed 16 months ago by greiner
Replying to kzar:
Well, this new string isn't located anywhere yet :p. Couldn't we just add it to adblockplusui, then migrate the other similar ones over in the future? (Yea, it would be kind of inconsistent.)
That should also work so, if necessary, we can do that for now.
comment:8 Changed 16 months ago by abpbot
A commit referencing this issue has landed:
Issue 6797 - Require active domains in snippet filters
comment:9 Changed 16 months ago by mjethani
- Description modified (diff)
comment:10 Changed 16 months ago by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
comment:11 Changed 16 months ago by sebastian
Fine, let's move all strings to adblockplusui.
comment:12 Changed 16 months ago by greiner
- Blocking 6846 added
comment:13 Changed 14 months ago by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Working as described. Snippets and emulation filters require an active domain.
ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10
Thank you very much for the heads-up about the new strings.
Unfortunately, they're still located in adblockpluschrome. Therefore it'd be great if we could get Platform's approval on #6633 so that we can take over management of those strings from them. After that has happened I can go on and create a UI ticket to introduce those new strings.