Opened on 06/01/2017 at 10:55:22 AM

Closed on 06/01/2017 at 02:45:36 PM

#5287 closed defect (fixed)

Change syntax for element hiding emulation filters and remove simplified element hiding syntax

Reported by: trev Assignee: trev
Priority: P2 Milestone:
Module: Core Keywords:
Cc: hfiguiere, Lain_13, fanboy, mapx Blocked By:
Blocking: #3143 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29453590/

Description (last modified by trev)

Background

We are currently using rather awkward syntax for element hiding filters, which also happens to be not too extensible. This is an obstacle for #3143 and follow-up issues.

What to change

  • Introduce a new syntax specifically for element hiding emulation filters:
    example.com#?#foo :-abp-properties(background-color:#fff) bar
    
    Note #?# instead of ## indicating that this isn't a regular element hiding filter but that the selector is rather being evaluated by Adblock Plus and some additional pseudo-classes are allowed.
  • Make filterClasses module translate legacy filters like example.com##foo [-abp-properties="background-color:#fff"] bar into example.com#?#foo :-abp-properties(background-color:#fff) bar "on the fly." The list should show the modified filter so that people stop using the legacy syntax ASAP. It's probably best to modify Filter.fromText() for that (otherwise Filter.fromText() might cache two copies of the same filter).
  • Make sure that all element hiding emulation filters have at least one :-abp-foo() pseudo-class, filters with plain selectors should be rejected as invalid. This should prevent people from using element hiding emulation rules by mistake.
  • While at it, remove support for simplified element hiding syntax, this should simplify the implementation considerably.

Integration notes

  • New filter_elemhideemulation_plainselector string has been added. It should say: "Cannot use plain selectors with element hiding emulation filters"
  • The strings filter_elemhide_duplicate_id and filter_elemhide_nocriteria should be removed, these refer to the simplified element hiding syntax.

Attachments (0)

Change History (5)

comment:1 Changed on 06/01/2017 at 11:02:30 AM by trev

  • Cc hfiguiere added
  • Description modified (diff)

comment:2 Changed on 06/01/2017 at 11:29:17 AM by mapx

  • Cc Lain_13 fanboy mapx added

comment:3 Changed on 06/01/2017 at 01:37:56 PM by Lain_13

Why do we have to keep that awkward smile :-a in each and every special hiding filter if we going to use #?# to mark them as special anyway? Except for: "This should prevent people from using element hiding emulation rules by mistake." Just check for presence of specific pseudoclasses like :properties() or :has().
I'm not even sure why do we need #?# in the first place except to speed-up parsing a little bit. If we really need this then why not to implement inline styles (#756) as well? With strict set of properties and values, ofc. It's really needed since we already have a lot of sites with branding and we can fix empty space left from it only on a little few. In some cases it's not possible at all (when path to background image is the same as to the all the other images on the page).

Last edited on 06/01/2017 at 02:01:23 PM by Lain_13

comment:4 Changed on 06/01/2017 at 02:44:51 PM by abpbot

comment:5 Changed on 06/01/2017 at 02:45:36 PM by trev

  • Resolution set to fixed
  • Review URL(s) modified (diff)
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from trev.
 
Note: See TracTickets for help on using tickets.