Move CSS escaping from ElemHideBase to ElemHide

[mjethani]

Background

We want to use the element hiding exception syntax for snippet filters as well (see #6538).

example.com#$#inject-css "body { background: white; }"
foo.example.com#@#inject-css "body { background: white; }"

But CSS escaping in ElemHideBase makes this difficult, because any { or } characters in the exception would get escaped, even though they are not escaped in the original snippet filter; thus the exception would not match the original filter.

On the other hand, the escaping of { and } at the time of filter initialization is a bit misplaced. It should be done at the time of injecting the selector into the document.

I propose that we move the escaping of the selector either directly into the web extension or at least into the ElemHide module, which deals directly with CSS selectors (for ElemHideEmulation also see #6665). Ideally the escaping would be done with ​CSS.escape, but for performance reasons.

What to change

Remove the CSS escaping from ElemHideBase.

Add CSS escaping to ElemHide.getSelectorsForDomain.

Resolution

In the initial Snippets implementation (#6781), script and selector are two different properties; the latter is an escaped version of the former. Therefore, there is no need to move the CSS escaping out of the filter classes, for now, as exceptions can be checked against the script property instead of the selector property.

This issue is closed as rejected.

[mjethani]

@sebastian @kzar what do you think about this?

[mjethani]

When you need curly braces in a script injection filter:

https://issues.adblockplus.org/ticket/6668#comment:9

mirsegondya.com##script:inject(abort-current-inline-script.js, String.fromCharCode, /\/\*[0-9a-f]{40}\*\//)

[kzar]

Replying to mjethani:

@sebastian @kzar what do you think about this?

Well honestly I don't know.

I had a look and it seems this change was originally introduced by Felix along with Wladimir a couple of years ago, there's some useful discussion about it in #4684. You might want to read through that, or ask Felix for some context before you make a decision.

[sebastian]

Replying to mjethani:

preferably using ​CSS.escape.

For reference, CSS.escape() isn't available in Microsoft Edge (yet).

[mjethani]

It seems there's a ​polyfill that we could use on Edge.

[mjethani]

I've removed the CSS.escape part from this issue.

For most selectors (from generic hiding filters), the escaping would have to be done only once in ElemHide. For selectors from domain-specific filters, it would have to be done on each call to ElemHide.getSelectorsForDomain. It should be OK if we limit it to just replacing { and } as we are doing in ElemHideBase now.

By the way I think we may not need this change, because ElemHideBase can take the script property from the superclass and assign its own selector property based on that (see ​latest patch if interested); the exceptions can still use the unescaped script property, while the selector property (escaped version of the script property) will be used for CSS injection.