Opened on 02/15/2019 at 02:18:01 PM

Closed on 02/22/2019 at 09:37:43 PM

Last modified on 04/24/2020 at 07:54:15 AM

#7291 closed defect (fixed)

Exception in element hiding emulation

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

https://codereview.adblockplus.org/30012569

Description (last modified by hfiguiere)

Environment

Chrome or Firefox

How to reproduce

  1. Disable all filters.
  2. Add the following filter wp.pl#?#div:-abp-properties(box-sizing: content-box;):-abp-has(a > div)
  3. Visit https://finanse.wp.pl/tasmy-kaczynskiego-mec-jacek-dubois-kaczynski-musi-zostac-przesluchany-6349714442750081v

Observed behaviour

In the web console there is an exception:

elemHideEmulation.js:189 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Document': '._3bcRuc4 div*' is not a valid selector.
    at scopedQuerySelector (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:789:24)
    at scopedQuerySelectorAll (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:795:10)
    at HasSelector.getElements (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:913:20)
    at getElements.next (<anonymous>)
    at HasSelector.getSelectors (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:898:14)
    at getSelectors.next (<anonymous>)
    at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:828:30)
    at evaluate.next (<anonymous>)
    at evaluate (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/include.preload.js:834:14)
    at evaluate.next (<anonymous>)

Expected behaviour

No exception

Hint to testers

You can test this on one or our testpages.

  1. Add the filter adblockplus.org#?#div:-abp-properties(float: left;)
  2. Visit https://testpages.adblockplus.org/en/filters/element-hiding-emulation
  3. If the bug is here, there is an exception in the console:
    include.preload.js:1407 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Document': '#site-header-logo > div*' is not a valid selector.
        at processPatterns (chrome-extension://fagebjibnhbcaljjbhoialonelckpden/include.preload.js:1407:45)
        at processPatterns (chrome-extension://fagebjibnhbcaljjbhoialonelckpden/include.preload.js:1420:14)
        at ElemHideEmulation._addSelectors (chrome-extension://fagebjibnhbcaljjbhoialonelckpden/include.preload.js:1423:5)
        at HTMLDocument.handler (chrome-extension://fagebjibnhbcaljjbhoialonelckpden/include.preload.js:1509:14)
    
  4. If fixes, the logo should disappear at the top.

Attachments (0)

Change History (9)

comment:1 Changed on 02/15/2019 at 02:19:00 PM by hfiguiere

  • Description modified (diff)

comment:2 Changed on 02/20/2019 at 08:23:13 PM by hfiguiere

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

comment:3 Changed on 02/21/2019 at 03:21:26 PM by mjethani

  • Description modified (diff)
  • Priority changed from Unknown to P2

comment:4 Changed on 02/21/2019 at 03:23:49 PM by mjethani

This ticket is OK except there needs to be a standalone test case so the bug/fix can be verified even if the original document has changed.

@hfiguiere can you make this reproducible without referring to external content not under our control?

In the Expected behavior section, we need to also clarify which elements should be hidden.

comment:5 Changed on 02/21/2019 at 05:50:39 PM by abpbot

A commit referencing this issue has landed:
Issue 7291 - Don't mangle selectors with a '*'

comment:6 Changed on 02/22/2019 at 05:55:11 PM by abpbot

A commit referencing this issue has landed:
Issue 7291 - Handle *.foo in qualifySelector()

comment:7 Changed on 02/22/2019 at 09:37:43 PM by hfiguiere

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

Added a fixed testcase in the "Hints for tester" section.

Ideally we would craft a test case. It requires to have in the stylesheet a rules that ends with a '*'.

comment:8 Changed on 07/25/2019 at 02:18:52 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

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

comment:9 Changed on 04/24/2020 at 07:54:15 AM by mjethani

  • Sensitive unset

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from hfiguiere.
 
Note: See TracTickets for help on using tickets.