Opened on 12/01/2016 at 06:40:35 PM

Closed on 12/13/2016 at 04:35:40 PM

Last modified on 05/22/2017 at 12:14:24 PM

#4684 closed defect (fixed)

Element hiding filters containing { or } in the selector part do not work

Reported by: fhd Assignee: fhd
Priority: P2 Milestone:
Module: Core Keywords:
Cc: trev, scuturic, amrmak, arthur Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: yes
Review URL(s):

Description (last modified by fhd)


ABP for Firefox 2.8.2

How to reproduce

  1. Go to filter preferences.
  2. Add a custom element hiding filter containing { or }, e.g.:[-abp-properties="/margin: [0-9]{2}px/"]

Observed behaviour

All spaces are being removed from the filter (as a side effect of it not being detected as an element hiding filter) and it doesn't actually work (since it fails the validity check).

Expected behaviour

The filter shows up in the list just as it was typed in, and it actually works.

Attachments (0)

Change History (8)

comment:1 Changed on 12/01/2016 at 06:42:42 PM by fhd

  • Description modified (diff)

comment:2 Changed on 12/01/2016 at 07:01:33 PM by fhd

The source of this issue is basically elemhideRegExp which doesn't match the filter above, because of it specifically not allowing { or } in the selector part: ([^{}].*). Would be easy enough to fix by not disallowing these characters but I wonder why we exclude those specifically.

I dug back into March 2006, and this got introduced along with non-simplified element hiding filters:

So I have no idea why we exclude those specifically, we don't worry about the validity of selectors otherwise.

comment:3 Changed on 12/01/2016 at 07:06:41 PM by trev

  • Cc trev added; removed

We exclude those specifically because not disallowing them might lead to rule injection - consider ##foo{color:red}. Normally, { and } in selectors are supposed to be escaped (\00007b and \00007d should work). So question is whether we want to support escape sequences within CSS property filters or solve it by different means.

comment:4 Changed on 12/02/2016 at 11:45:52 PM by fhd

  • Owner set to fhd

My vote goes to "other means" - regexp matching is tricky enough, we should probably not make it even trickier. Generally speaking, we should allow arbitrary content in the value part of both attribute and property selectors, but still disallow it everywhere else. I'll look into that.

comment:5 Changed on 12/08/2016 at 10:14:49 AM by fhd

  • Cc scuturic amrmak arthur added
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:6 Changed on 12/13/2016 at 04:34:50 PM by abpbot

A commit referencing this issue has landed:
Issue 4684 - Allow { and } in attribute and property selectors

comment:7 Changed on 12/13/2016 at 04:35:40 PM by fhd

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:8 Changed on 05/22/2017 at 12:14:24 PM by Ross

  • Verified working set

Tested this change as part of #4658. Using {} in filters now works as expected.

Firefox 51 / 53 / Beta (54.0b2)
Windows 7

Add Comment

Modify Ticket

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