Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#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.

Change History (8)

comment:1 Changed 4 years ago by fhd

  • Description modified (diff)

comment:2 Changed 4 years ago 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 4 years ago by trev

  • Cc trev added; trev@… 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 4 years ago 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 4 years ago by fhd

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

comment:6 Changed 4 years ago by abpbot

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

comment:7 Changed 4 years ago by fhd

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

comment:8 Changed 3 years ago 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

Note: See TracTickets for help on using tickets.