Opened on 12/12/2017 at 05:37:11 PM
Closed on 04/20/2018 at 04:26:39 PM
Last modified on 07/12/2019 at 08:16:27 AM
#6193 closed change (fixed)
Applying exceptions to advanced pseudo-selector rules
| Reported by: | amrmak | Assignee: | hfiguiere |
|---|---|---|---|
| Priority: | P2 | Milestone: | Adblock-Plus-3.1-for-Chrome-Opera-Firefox |
| Module: | Platform | Keywords: | |
| Cc: | hfiguiere, mapx, sebastian, kzar, arthur, mjethani, sergz | Blocked By: | |
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by kzar)
Environment
Windows 10 10.0.16299 (64-bit)
Chrome 63.0.3239.84 (64-bit) (same with Firefox 57.0.2 64-bit)
Adblock Plus 1.13.4 (same with ABP 3.0.2)
How to reproduce
- Go to Adblock Plus options page, and go to "Add your own filters tab"
- In the first text box, add the filter planet.fr#@#:-abp-properties(cursor: pointer;)
- Click "Add filter"
Observed behaviour
An Adblock Plus alert is triggered with the message:
"Line 1: ':-abp-properties(cursor: pointer;)' is not a valid CSS selector"
Expected behaviour
No alerts, and filter is added to custom filters.
Attachments (0)
Change History (21)
comment:1 Changed on 12/12/2017 at 05:37:44 PM by amrmak
- Platform changed from Unknown / Cross platform to Chrome
comment:2 Changed on 12/12/2017 at 05:39:12 PM by amrmak
- Platform changed from Chrome to Unknown / Cross platform
comment:3 Changed on 12/12/2017 at 05:41:52 PM by hfiguiere
- Cc hfiguiere added; hubert@adblockplus.org removed
comment:4 Changed on 12/12/2017 at 05:45:16 PM by hfiguiere
- Component changed from Core to Platform
comment:5 follow-up: ↓ 8 Changed on 12/12/2017 at 06:21:41 PM by mapx
comment:6 Changed on 12/12/2017 at 06:22:55 PM by mapx
- Cc mapx added
comment:7 Changed on 12/12/2017 at 06:35:34 PM by hfiguiere
- Cc sebastian kzar added
comment:8 in reply to: ↑ 5 ; follow-up: ↓ 12 Changed on 12/12/2017 at 06:52:58 PM by hfiguiere
Replying to mapx:
this 1 is accepted
planet.fr#@?#:-abp-properties(cursor:pointer;)
This looks like a bug in core. It is parsed as a blocking filter.
comment:9 Changed on 12/12/2017 at 06:55:15 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:10 Changed on 12/12/2017 at 06:56:14 PM by mapx
Is it necessary having that ugly form #?# ?
Why not just ## as for the normal hiding filters ?
The emulation filters are recognized using :-abp
comment:11 Changed on 12/13/2017 at 08:37:15 AM by arthur
- Cc arthur added
comment:12 in reply to: ↑ 8 ; follow-ups: ↓ 13 ↓ 14 Changed on 01/08/2018 at 03:19:39 PM by kzar
- Cc sergz mjethani added
- Component changed from Platform to Core
- Description modified (diff)
- Type changed from defect to change
Since this issue was marked as a defect under the wrong module and the review gave no / little more context I spent some time figuring out what it was about. Please could you give some more context in the review and issue next time, so that someone who isn't up to speed can get the idea more quickly? Also changing filter syntax requires some coordination and discussion.
The #@# syntax seems to be for regular element hiding exceptions, as in the opposite of a ## filter. By that logic shouldn't exceptions for advanced pseudo-selector rules have the syntax #@?#? Otherwise like mapx suggestions why do we both with the special #?# syntax at all, if we're going to check the filter for the string ":-abp-" anyway?
Replying to hfiguiere:
Replying to mapx:
this 1 is accepted
planet.fr#@?#:-abp-properties(cursor:pointer;)This looks like a bug in core. It is parsed as a blocking filter.
Have you filed an issue for that?
comment:13 in reply to: ↑ 12 Changed on 01/08/2018 at 04:51:38 PM by hfiguiere
Replying to kzar:
Since this issue was marked as a defect under the wrong module [...]
It really is a defect in the addon, not core. The problem is in the filter validation when added from the UI. The patch in review is fixing just that.
The #@# syntax seems to be for regular element hiding exceptions, as in the opposite of a ## filter. By that logic shouldn't exceptions for advanced pseudo-selector rules have the syntax #@?#? Otherwise like mapx suggestions why do we both with the special #?# syntax at all, if we're going to check the filter for the string ":-abp-" anyway?
It was decided that the exceptions filter applied to both ElementHiding and ElementHidingEmulation, hence no different syntax.
comment:14 in reply to: ↑ 12 Changed on 01/08/2018 at 04:57:56 PM by hfiguiere
Replying to kzar:
Replying to hfiguiere:
Replying to mapx:
this 1 is accepted
planet.fr#@?#:-abp-properties(cursor:pointer;)This looks like a bug in core. It is parsed as a blocking filter.
Have you filed an issue for that?
Filed https://issues.adblockplus.org/ticket/6234 now.
comment:15 Changed on 01/09/2018 at 12:09:39 PM by kzar
- Cc sergz removed
- Component changed from Core to Platform
It really is a defect in the addon, not core. The problem is in the filter validation when added from the UI. The patch in review is fixing just that.
OK sorry, I've changed the module back.
It was decided that the exceptions filter applied to both ElementHiding and ElementHidingEmulation, hence no different syntax.
Could you provide some more detail? (E.g. how it was discussed to come to that decision.)
comment:16 Changed on 01/09/2018 at 04:39:13 PM by hfiguiere
It was not explicitely discussed. Wladimir patch for https://issues.adblockplus.org/ticket/5287 was deliberately using the same syntax for exception. I don't see the need for a different syntax either.
comment:17 Changed on 01/10/2018 at 11:31:31 AM by kzar
- Priority changed from Unknown to P2
- Ready set
Ah yea, you're right. Fair enough, I'll set this as ready.
comment:18 Changed on 02/13/2018 at 09:25:49 AM by sergz
- Cc sergz added
comment:19 Changed on 04/20/2018 at 04:25:43 PM by abpbot
A commit referencing this issue has landed:
Issue 6193 - Allow element hiding emulation exceptions
comment:20 Changed on 04/20/2018 at 04:26:39 PM by hfiguiere
- Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
- Resolution set to fixed
- Status changed from reviewing to closed
comment:21 Changed on 05/10/2018 at 09:57:05 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed. Element hiding emulation exceptions can be added and are functional.
ABP 3.0.4.2042
Firefox 59 / 55 / 51 / Windows 10
Chrome 66 / 58 / 49 / Windows 7
Opera 52 / 45 / 36 / Windows 10
this 1 is accepted