Opened on 01/21/2018 at 04:10:19 PM
Closed on 02/21/2018 at 03:22:14 PM
Last modified on 03/20/2018 at 02:45:24 PM
#6296 closed defect (fixed)
Procedural hiding stops working when ...:-abp-has(> ...:-abp-contains(...)) is used
Reported by: | Lain_13 | Assignee: | hfiguiere |
---|---|---|---|
Priority: | P2 | Milestone: | |
Module: | Core | Keywords: | |
Cc: | hfiguiere, mjethani, kzar, mapx, sergz | Blocked By: | |
Blocking: | Platform: | Unknown / Cross platform | |
Ready: | yes | Confidential: | no |
Tester: | Ross | Verified working: | yes |
Review URL(s): |
Description
Environment
Google Chrome 63.0.3239.132
Adblock Plus development build 3.0.2.1953
How to reproduce
- Add filter litlib.net#?#tr:-abp-has(> td[class^="content"]:-abp-contains(Реклама))
- Visit page: https://www.litlib.net/tags
Another example of the same error:
- Add filter example.com#?#div:-abp-has(> span:-abp-contains(Advertisment)) from this page: https://adblockplus.org/filter-cheatsheet
- Visit http://example.com/ and look in the console for the same error.
Observed behaviour
Empty header with text "РЕКЛАМА:" is visible above the block "ТЕГИ:" in the middle of the page.
Expected behaviour
Block with header "РЕКЛАМА:" (and that header) is hidden.
Error in console
elemHideEmulation.js:243 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Element': '> *' is not a valid selector. at ContainsSelector.getElements (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:956:28) at getElements.next (<anonymous>) at ContainsSelector.getSelectors (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:948:14) at getSelectors.next (<anonymous>) at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:845:30) at evaluate.next (<anonymous>) at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:851:14) at evaluate.next (<anonymous>) at HasSelector.getElements (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:914:16) at getElements.next (<anonymous>)
As I understand code there lacks special treatments for cases when >, + or ~ are used at the start to indicate that element should be either direct child or neighbor of current element.
include.preload.js source with debug:
ContainsSelector.prototype = { requiresHiding: true, *getSelectors(prefix, subtree, stylesheet) { for (let element of this.getElements(prefix, subtree, stylesheet)) yield [makeSelector(element, ""), subtree]; }, *getElements(prefix, subtree, stylesheet) // debug: prefix = "> td[class^="content"]", subtree = tr, stylesheet = Array(298) { let actualPrefix = (!prefix || incompletePrefixRegexp.test(prefix)) ? // debug: actualPrefix = "> td[class^="content"]", prefix = "> td[class^="content"]" prefix + "*" : prefix; let elements = subtree.querySelectorAll(actualPrefix); // crash here due to incomplete actualPrefix for (let element of elements) { if (element.textContent.includes(this._text)) yield element; else yield null; } } };
Attachments (0)
Change History (16)
comment:1 Changed on 01/22/2018 at 02:02:02 PM by mapx
- Cc hfiguiere mjethani kzar mapx added
comment:2 Changed on 01/22/2018 at 05:28:05 PM by hfiguiere
- Owner set to hfiguiere
comment:3 Changed on 01/22/2018 at 05:53:11 PM by Lain_13
As I understand > should work with :scope on the left side.
Example (run on the https://www.litlib.net/tags):
let node; node = document.querySelector('.content_c').closest('table').closest('tr'); // returns: <tr>…</tr> node.querySelector(':scope > .content_c') // since .content_c is not a direct child of the :scope the result is 'null' node = document.querySelector('.content_c').closest('tr'); // this tr is direct parent of .content_c: <tr style="display: none !important;">…</tr> node.querySelector(':scope > .content_c') // result: <td class="content_c">Реклама: </td>
That won't work with + and ~, though. They are addressing elements outside of the current scope. So, generating selector for the current node might be a necessity. Actually, even node.querySelector() won't work in such case since it's scoped to the node. You'll have to call it at least from a parent node or stright from a document (depending on how you construct an unique selector for the current node).
comment:4 follow-up: ↓ 7 Changed on 01/22/2018 at 09:01:29 PM by hfiguiere
+ and ~ do not make sense inside a :-abp-has() since it is implicitly about children.
comment:5 Changed on 01/22/2018 at 09:27:22 PM by hfiguiere
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:6 Changed on 01/22/2018 at 09:38:00 PM by hfiguiere
- Component changed from Unknown to Core
comment:7 in reply to: ↑ 4 Changed on 01/23/2018 at 01:04:03 AM by Lain_13
Replying to hfiguiere:
True, I somehow forgot that it's for :-abp-has().
comment:8 Changed on 01/23/2018 at 10:11:04 AM by kzar
- Cc sergz added
- Priority changed from Unknown to P2
- Ready set
comment:9 Changed on 02/01/2018 at 03:37:21 PM by abpbot
A commit referencing this issue has landed:
Issue 6296 - Handle relative prefix in :-abp-has()
comment:10 Changed on 02/01/2018 at 03:37:48 PM by hfiguiere
- Resolution set to fixed
- Status changed from reviewing to closed
comment:11 Changed on 02/21/2018 at 11:56:28 AM by Lain_13
- Resolution fixed deleted
- Status changed from closed to reopened
ABP (1.13.5 and 3.0.2.1974 (dev))
Google Chrome 64.0.3282.167 (Official Build) (64-bit)
Domain: stratege.ru
Filter: stratege.ru#?#div[class$="_box"]:-abp-has(> div[class*="title"]:-abp-contains(Реклама))
Log:
elemHideEmulation.js:243 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Element': '> div[class*="title"]' is not a valid selector. at ContainsSelector.getElements (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:972:28) at getElements.next (<anonymous>) at ContainsSelector.getSelectors (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:964:14) at getSelectors.next (<anonymous>) at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:861:30) at evaluate.next (<anonymous>) at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:867:14) at evaluate.next (<anonymous>) at HasSelector.getElements (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:930:16) at getElements.next (<anonymous>)
A commit referencing this issue has landed:
Issue 6296 - Handle relative prefix in :-abp-has()
As I understand code related to this fix were moved to HasSelector.prototype.getElements, while the issue actually happens in the ContainsSelector.prototype.getElements.
comment:12 Changed on 02/21/2018 at 02:04:53 PM by hfiguiere
The thing is that adblockpluscore hasn't been updated in that development build of Adblock Plus so the fix you are expecting isn't even there.
comment:13 Changed on 02/21/2018 at 02:38:23 PM by Lain_13
Strange, I thought it's already there. Now I see it's not even in the changelog of dev build. Sorry about that. Please close this issue again.
comment:14 Changed on 02/21/2018 at 03:22:14 PM by hfiguiere
- Resolution set to fixed
- Status changed from reopened to closed
comment:15 Changed on 02/22/2018 at 05:45:56 PM by hfiguiere
I have filed issue #6412 to update the dependencies and have this change brought over to the WebExtension.
comment:16 Changed on 03/20/2018 at 02:45:24 PM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Fixed.
A version of this 127.0.0.1#?#div:-abp-has(> span:-abp-contains(Advertisment)) against a test page with the element worked as expected. I couldn't reproduce the example using litlib.net above.
ABP 3.0.2.1983
Firefox 51 / 58 / Windows 10
Chrome 49 / 65 / Windows 7
Opera 36 / 51 / Windows 7
This should work and it doesn't. Looking at it.