Opened 2 weeks ago

Closed 2 weeks ago

Last modified 9 days ago

#7137 closed change (fixed)

Implement hide-if-has-and-matches-style snippet

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

https://codereview.adblockplus.org/29952555/

Description (last modified by mjethani)

Background

This snippet should be similar to the hide-if-contains-and-matches-style snippet (#6919), except the first parameter should be a CSS selector. If an element has any descendants that match the CSS selector, it should be considered a candidate for being hidden.

What to change

Implement a snippet hide-if-has-and-matches-style with the following signature:

function hideIfHasAndMatchesStyle(search, selector = "*",
                                  searchSelector = null, style = null,
                                  searchStyle = null)

All the parameters except the first one should have the same meaning as in hide-if-contains-and-matches-style (#6919).

The first parameter, instead of being any arbitrary string or a regular expression, should be a CSS selector. If any element contains a descendant that matches the CSS selector, such that !!element.querySelector(search) is true, then it should satisfy the first condition. The rest of the semantics should be exactly as hide-if-contains-and-matches-style (#6919).

Hints for testers

Try this:

<!-- test.html -->
<div>
  Lorem ipsum dolor sit amet, consectetur
  adipiscing elit, sed do eiusmod tempor
  incididunt ut labore et dolore magna aliqua.
</div>
<script src="script.js"></script>
/* script.js */
setTimeout(() =>
{
  let ad = document.createElement("div");
  ad.id = Math.random().toString(36).substring(2);
  ad.innerHTML = "<div><span>Check this!</span><span class='label'><a href='#ads'>Ad</a></span></div>";
  document.body.appendChild(ad);

  let result = document.createElement("div");
  result.id = Math.random().toString(36).substring(2);
  result.innerHTML = "<div><span>The Adventures of Tom Sawyer</span><span class='label' style='display: none'><a href='#ads'>Ad</a></span></div>";
  document.body.appendChild(result);
},
500);

The following filters, each one on its own, should hide the "Check this!" ad and not the "The Adventures of Tom Sawyer" content:

  • #$#hide-if-has-and-matches-style a[href="#ads"] div[id] span.label /./ 'display: inline;'
  • #$#hide-if-has-and-matches-style a[href="#ads"] div[id] span.label ';' /\\bdisplay:\ inline\;/

Change History (15)

comment:1 Changed 2 weeks ago by mjethani

  • Review URL(s) modified (diff)

comment:2 Changed 2 weeks ago by arthur

  • Cc arthur added

comment:3 Changed 2 weeks ago by mjethani

  • Description modified (diff)

comment:4 Changed 2 weeks ago by mjethani

  • Cc hfiguiere Ross added
  • Description modified (diff)

comment:5 Changed 2 weeks ago by mjethani

  • Description modified (diff)
  • Status changed from new to reviewing

comment:6 Changed 2 weeks ago by mjethani

  • Ready set

comment:7 Changed 2 weeks ago by abpbot

A commit referencing this issue has landed:
Issue 7137 - Implement hide-if-has-and-matches-style snippet

comment:8 Changed 2 weeks ago by mjethani

  • Owner set to mjethani

comment:9 Changed 2 weeks ago by mjethani

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

comment:10 Changed 2 weeks ago by abpbot

A commit referencing this issue has landed:
Issue 7137 - Implement hide-if-has-and-matches-style snippet

comment:11 Changed 9 days ago by Ross

This seems to be working as described. However I could not get the first filter example to work with the example given and cannot figure out why (or if it's an issue)...

ABP 3.4.1.2204
Chrome 70 / 49 / Windows 10
Firefox 63 / 51 / Windows 10

comment:12 Changed 9 days ago by mjethani

  • Description modified (diff)

comment:13 Changed 9 days ago by mjethani

I missed the closing quote in that first filter. It should work now.

comment:14 Changed 9 days ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Ah, thank you. This is fine then.

ABP 3.4.1.2204
Chrome 70 / 49 / Windows 10
Firefox 63 / 51 / Windows 10

comment:15 Changed 9 days ago by mjethani

  • Sensitive unset
Note: See TracTickets for help on using tickets.