Opened 20 months ago

Last modified 19 months ago

#6847 closed change

Add regular expression support to hide-if-contains and hide-if-shadow-contains snippets — at Version 6

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

https://codereview.adblockplus.org/29852577/

Description (last modified by mjethani)

Background

Currently the hide-if-contains and hide-if-shadow-contains snippets accept a search string as their first argument. This works when the text is fixed and sufficiently unique, but it doesn't work when the text is more generic and can appear in non-ad items.

For example, the filter #$#hide-if-contains ad li can end up hiding elements that contain the word "adventure" or "advocacy", which could be part of the content of the document.

What to change

Let hide-if-contains and hide-if-shadow-contains treat the search string as a regular expression if the string is surrounded by /.

Hints for testers

Test that both hide-if-contains and hide-if-shadow-contains work as described in #6809 and #6798 respectively (i.e. no regressions).

For this change, try the following test:

<!-- 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'>Ad</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></div>";
  document.body.appendChild(result);
},
500);

The filter #$#hide-if-contains /\\bAd\\b/ div[id] should hide the "Check this!" ad and not the "The Adventures of Tom Sawyer" content.

Also try the following test:

<!-- test.html -->
<!-- Content --> 
<div>
  Lorem ipsum dolor sit amet, consectetur
  adipiscing elit, sed do eiusmod tempor
  incididunt ut labore et dolore magna aliqua.
</div>
<!-- Ad -->
<div id="qvdqv3k4hxe">
  <div>Best ad ever!</div>
  <div></div>
</div>
<!-- Advocacy -->
<div id="zc34r93jglg">
  <div>Support the Open Web</div>
  <div></div>
</div>
<script src="script.js"></script>
/* script.js */
setTimeout(() =>
{       
  let labelElement =
    document.getElementById("qvdqv3k4hxe").lastElementChild;
  let shadowRoot =
    labelElement.attachShadow({mode: "closed"});
  let span = document.createElement("span");
  span.innerText = "Ad";
  shadowRoot.appendChild(span);

  labelElement =
    document.getElementById("zc34r93jglg").lastElementChild;
  shadowRoot =
    labelElement.attachShadow({mode: "closed"});
  span = document.createElement("span");
  span.innerText = "Advocacy";
  shadowRoot.appendChild(span);
},
500);

The filter #$#hide-if-shadow-contains /\\bAd\\b/ div[id] should hide the ad "Best ad ever!" but not the content "Support the Open Web".

Change History (6)

comment:1 Changed 20 months ago by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:2 Changed 20 months ago by mjethani

  • Owner set to mjethani
  • Review URL(s) modified (diff)

comment:3 Changed 20 months ago by mjethani

  • Status changed from new to reviewing

comment:4 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 6847 - Add regular expression support to snippets

comment:5 Changed 20 months ago by mjethani

  • Description modified (diff)

comment:6 Changed 20 months ago by mjethani

  • Description modified (diff)
Note: See TracTickets for help on using tickets.