Opened on 07/19/2018 at 01:41:16 PM
Closed on 08/08/2018 at 03:36:11 PM
Last modified on 10/23/2018 at 02:27:17 PM
#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.
Attachments (0)
Change History (13)
comment:2 Changed on 08/07/2018 at 02:52:21 PM by mjethani
comment:3 Changed on 08/07/2018 at 02:54:33 PM by mjethani
- Owner set to mjethani
comment:5 Changed on 08/07/2018 at 04:21:08 PM by greiner
- Cc wspee greiner added
comment:6 follow-up: ↓ 7 Changed on 08/08/2018 at 10:54:18 AM 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 on 08/08/2018 at 12:17:06 PM 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 on 08/08/2018 at 03:26:09 PM by abpbot
A commit referencing this issue has landed:
Issue 6797 - Require active domains in snippet filters
comment:10 Changed on 08/08/2018 at 03:36:11 PM by mjethani
- Resolution set to fixed
- Status changed from reviewing to closed
comment:11 Changed on 08/08/2018 at 06:22:57 PM by sebastian
Fine, let's move all strings to adblockplusui.
comment:12 Changed on 08/09/2018 at 10:40:20 AM by greiner
- Blocking 6846 added
comment:13 Changed on 10/23/2018 at 02:27:17 PM 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.