Opened 16 months ago

Closed 16 months ago

Last modified 15 months ago

#6809 closed change (fixed)

Implement hide-if-contains snippet

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

https://codereview.adblockplus.org/29836573/

Description (last modified by mjethani)

Background

The problem is described in #6804.

Until this improvement is made to element hiding emulation, we should use a snippet to hide an ad before the user sees it.

What to change

Implement a snippet called hide-if-contains with the following signature:

function hideIfContains(search, selector = "*")

On any DOM modification, the snippet should use document.querySelectorAll(selector) to look up all elements that match the given CSS selector, then look for search in each element's text content and hide the element if the string is found.

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>Best ad ever!</div>";
  document.body.appendChild(ad);
},
500);

The filter #$#hide-if-contains 'Best ad ever!' div[id] should hide the ad.

Change History (12)

comment:1 Changed 16 months ago by abpbot

A commit referencing this issue has landed:
Issue 6809 - Implement hide-if-contains snippet

comment:2 Changed 16 months ago by mjethani

  • Cc kzar hfiguiere added
  • Component changed from Unknown to Core
  • Description modified (diff)
  • Keywords circumvention added
  • Priority changed from Unknown to P2
  • Ready set
  • Review URL(s) modified (diff)

comment:3 Changed 16 months ago by mjethani

  • Description modified (diff)

comment:4 Changed 16 months ago by mjethani

  • Owner set to mjethani

comment:5 Changed 16 months ago by mjethani

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

comment:6 Changed 16 months ago by abpbot

A commit referencing this issue has landed:
Issue 6809 - Implement hide-if-contains snippet

comment:7 Changed 15 months ago by Ross

Appears to be working correctly in Firefox 62. However in Chrome 68 it only seems to work some of the time. Manish do you see the same?

comment:8 Changed 15 months ago by mjethani

  • Description modified (diff)

comment:9 Changed 15 months ago by mjethani

@Ross I've updated the test in this issue. There may be a timing issue, which has not been a problem in practice for us. I'll try to address it anyway as a separate issue. Meanwhile let's try with a 500-millisecond delay as shown in the example.

comment:10 Changed 15 months ago by Ross

Thanks. It then works as expected.

ABP 3.2.2103
Firefox 61 / 55 / 51 / Windows 10
Chrome 68 / 55 / 49 / Windows 10

comment:11 Changed 15 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

comment:12 Changed 15 months ago by mjethani

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