Opened 4 months ago

Closed 3 months ago

#5404 closed defect (fixed)

:-abp-properties() nested within :-abp-has() doesn't work if the relevant selector refers to outside nodes

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

https://codereview.adblockplus.org/29486558/

Description

Environment

Adblock Plus for Chrome 1.13.2.1786 development build

How to reproduce

  1. Add adblockplus.org#?##summary:-abp-has(:-abp-properties(color: rgb(51, 51, 51))) to your custom filters.
  2. Go to https://adblockplus.org/

Observed behaviour

The text above the download button (in particular, the feature list) is visible.

Expected behaviour

The text above the download button should be hidden.

Background

:-abp-properties() pseudo will produce some selectors, in this particular case #summary ul. The current implementation of :-abp-has() will currently combine these selectors with the selector for the candidate element, so that we are effectively testing #summary #summary ul - this selector doesn't have any matches because the #summary element isn't located within itself.

What to change

Don't combine selectors produced by :-abp-properties(), rather limit queryElement() call to the necessary element so that no elements outside of it are returned.

Change History (7)

comment:1 Changed 4 months ago by trev

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

comment:3 Changed 4 months ago by trev

Leaving open because this needs to land on master as well, so far this went to a branch only.

comment:4 Changed 4 months ago by Ross

Fixed.

ABP 1.13.2.1787
Chrome 51+ / Windows 7
Opera 38+ / Windows 7

comment:5 Changed 4 months ago by hfiguiere

  • Blocking 5436 added

comment:7 Changed 3 months ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed

Landed on master now, only context changed slightly here.

Note: See TracTickets for help on using tickets.