Opened 3 years ago

Closed 3 years ago

#4600 closed change (fixed)

Change CSS property matching to be case insensitive

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

https://codereview.adblockplus.org/29362481/

Description

Background

As discussed in the review for #4593, there is no real use case for matching CSS properties in a case sensitive way (as we currently do it), since most property names/values are case insensitive. In addition, filter expressions (whose syntax non-regexp property matching is based on) are generally case insensitive unless the match-case option is specified.

What to change

Match property selectors (both the original filter expressions and regular expressions introduced with #4593) in a case insensitive way.

Change History (7)

comment:1 Changed 3 years ago by fhd

  • Component changed from Unknown to Core

comment:2 Changed 3 years ago by fhd

What do you think Wladimir, do we want this? To me it makes sense: I can't think of a case where matching case sensitively would be important for the few case sensitive parts of CSS (e.g. the value of the content property, and URLs).

comment:3 Changed 3 years ago by trev

  • Blocked By 4593 removed
  • Priority changed from Unknown to P2
  • Ready set

First of all, CSS property values are case-sensitive. For example, content: "foo" isn't the same thing as content: "FOO". The risk here is generally false positives because we make the rules less precise.

On the other hand, case-sensitive rules make things complicated for filter list authors if letter case really doesn't matter. There is no easy way to match color: #abcdef in a case-insensitive way.

All things considered, it seems that case-insensitive matching makes more sense by default. We might want to introduce an option to override that default, but looking at EasyList and EasyList Germany there are currently no rules using $match-case blocking rule option whatsoever.

comment:4 Changed 3 years ago by fhd

From what I've read, all CSS property names and most CSS property values are case insensitive, see https://www.w3.org/TR/CSS2/syndata.html#characters. The only exceptions to this I'm aware of are URLs (since URLs are always case sensitive) and the value of the content property. Matching URLs in a case insensitive way makes sense to me, since URL filters do that as well. With content, I just don't see a use case for when the case would matter...

Selectors are a different matter, but that's not relevant here since we don't match those with CSS property filters.

comment:5 Changed 3 years ago by fhd

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:6 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4600 - Change CSS property matching to be case insensitive

comment:7 Changed 3 years ago by fhd

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.