Opened on 07/26/2018 at 12:17:21 PM

Closed on 07/26/2018 at 12:33:09 PM

Last modified on 09/06/2018 at 04:23:00 PM

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

Attachments (0)

Change History (12)

comment:1 Changed on 07/26/2018 at 12:18:28 PM by abpbot

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

comment:2 Changed on 07/26/2018 at 12:23:59 PM 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 on 07/26/2018 at 12:30:15 PM by mjethani

  • Description modified (diff)

comment:4 Changed on 07/26/2018 at 12:33:00 PM by mjethani

  • Owner set to mjethani

comment:5 Changed on 07/26/2018 at 12:33:09 PM by mjethani

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

comment:6 Changed on 08/02/2018 at 09:02:33 AM by abpbot

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

comment:7 Changed on 08/15/2018 at 01:41:19 PM 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 on 08/15/2018 at 09:52:17 PM by mjethani

  • Description modified (diff)

comment:9 Changed on 08/15/2018 at 09:53:21 PM 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 on 08/16/2018 at 01:11:41 PM 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 on 08/16/2018 at 01:11:53 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

comment:12 Changed on 09/06/2018 at 04:23:00 PM 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 mjethani.
 
Note: See TracTickets for help on using tickets.