Opened on 02/09/2019 at 12:39:40 AM
Closed on 02/14/2019 at 12:57:23 PM
Last modified on 07/25/2019 at 12:56:37 PM
#7268 closed defect (fixed)
ContainsSelector element hide emulation doesn't work with {} in regexp
| Reported by: | hfiguiere | Assignee: | hfiguiere |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Unknown / Cross platform | |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description (last modified by mjethani)
Environment
ABP 3.4.3
Any supported browser
Any supported OS
No filter lists
How to reproduce
- Create an advanced selector filter with :-abp-contains() and a regexp that contain a quantifier {n}. Something like
example.com#?#div:-abp-contains([aAbBdD]{9,}) - Create an HTML document <!DOCTYPE html><html><body><div>Badbadbad</div><div>Goodgoodgood</div></body></html>
Observed behaviour
The filter doesn't hide any elements.
Expected behaviour
The filter should hide the element containing the text Badbadbad.
Additional notes
ContainsSelector element hide emulation doesn't work with {} in regexp.
This is because in ElemHideBase.selector() we escape
{ and } to prevent CSS injection but the ContainsSelector constructor doesn't put them back.
Attachments (0)
Change History (17)
comment:1 Changed on 02/09/2019 at 12:39:53 AM by hfiguiere
- Owner set to hfiguiere
comment:2 Changed on 02/09/2019 at 12:43:06 AM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:3 Changed on 02/11/2019 at 12:55:03 AM by mjethani
comment:4 Changed on 02/11/2019 at 03:44:14 AM by mjethani
Let's start by adding a new escapedSelector property getter to replace the current selector property. The selector property should be used everywhere except in contexts where the caller needs to inject the selector into a style sheet.
comment:5 Changed on 02/11/2019 at 05:30:00 PM by hfiguiere
We can definitely do a more complex fix that will work. But at the moment this is a situation that :-abp-properties() handle but not :-abp-contains().
comment:6 Changed on 02/12/2019 at 12:40:51 PM by mjethani
What I meant was:
- We change the selector getter in lib/filterClasses.js to return the unescaped value of this.body
- In createStyleSheet in lib/elemHide.js, we escape the selector before writing it to the style sheet
- Remove the unescaping from lib/content/elemHideEmulation.js for PropsSelector
What do you think?
comment:7 Changed on 02/12/2019 at 11:52:15 PM by hfiguiere
Done
comment:9 Changed on 02/13/2019 at 06:04:43 PM by abpbot
A commit referencing this issue has landed:
Issue 7268 - Unescape { and } for :-abp-contains()
comment:10 Changed on 02/14/2019 at 02:59:41 AM by mjethani
- Description modified (diff)
- Priority changed from Unknown to P2
- Ready set
comment:11 Changed on 02/14/2019 at 03:02:38 AM by mjethani
- Description modified (diff)
comment:12 Changed on 02/14/2019 at 03:03:09 AM by mjethani
- Review URL(s) modified (diff)
comment:13 Changed on 02/14/2019 at 03:04:12 AM by mjethani
Let's close this, I'll file a new issue for the refactoring.
comment:14 Changed on 02/14/2019 at 04:01:49 AM by mjethani
Filed #7284.
comment:15 Changed on 02/14/2019 at 12:57:03 PM by hfiguiere
- Review URL(s) modified (diff)
comment:16 Changed on 02/14/2019 at 12:57:23 PM by hfiguiere
- Resolution set to fixed
- Status changed from reviewing to closed
comment:17 Changed on 07/25/2019 at 12:56:37 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Working as described.
ABP 0.9.15.2340
Microsoft Edge 44.17763.1.0 / Windows 10 1809
ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2
I think it's time to address the issue raised in #6726. We should move the escaping from the ElemHideBase class to the ElemHide module, since we are now generating the style sheet itself over there. It should be part of the createStyleSheet function. The selector property should contain the original raw selector.
The rules for CSS escaping are simple and can be written in less than 20 lines of JavaScript.