Opened 2 months ago

Closed 2 days ago

#7236 closed change (fixed)

Handle sub properties in abort-on-property-* snippets.

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

https://codereview.adblockplus.org/29995559/

Description (last modified by hfiguiere)

Background

Currently we only support a simple property (like property1) in abort-on-property-read and abort-on-property-write. We need to support any like object1.property1

What to change

  • Change wrapPropertyAccess() in lib/content/snippets.js to deal with that case.
  • Add unit tests.

Hint to testers

Read

Adapted from issue #6969

<!-- 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 */
adsOptions = {shown: false};

setTimeout(() =>
{  
  if (!adsOptions.shown)
  {
    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);

    adsOptions.shown = true;
  }
},
500);

When you load this page, it should show the "Check this!" ad. If you then add the filter localhost#$#abort-on-property-read adsOptions.shown and reload the page, the ad should no longer be shown. Also there should be no error in the tab's console.

Write

Adapted from issue #7207

Use the same HTML as above.

/* script.js */
setTimeout(() =>
{  
  if (!window.adsOptions || !window.adsOptions.showing)
  {
    if (!window.adsOptions || typeof window.adsOptions != "object")
      window.adsOptions = {};
    window.adsOptions.showing = true;
    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);
  }
},
500);

When you load this page, it should show the "Check this!" ad. If you then add the filter localhost#$#abort-on-property-write adsOptions.showing and reload the page, the ad should no longer be shown. Also there should be no error in the tab's console.

Change History (13)

comment:1 Changed 2 months ago by hfiguiere

  • Cc mjethani added

comment:2 Changed 2 months ago by arthur

  • Cc arthur added

comment:3 Changed 8 weeks ago by hfiguiere

  • Keywords circumvention added

comment:4 Changed 7 weeks ago by hfiguiere

  • Owner set to hfiguiere

comment:5 Changed 7 weeks ago by hfiguiere

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

comment:6 Changed 7 weeks ago by hfiguiere

  • Description modified (diff)

comment:7 Changed 2 weeks ago by amrmak

  • Cc amr added

comment:8 Changed 2 days ago by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:9 Changed 2 days ago by mjethani

Let's add some hints for testers here before we close the ticket.

comment:10 Changed 2 days ago by abpbot

A commit referencing this issue has landed:
Issue 7236 - Handle sub properties in abort-on-property snippets

comment:11 Changed 2 days ago by abpbot

A commit referencing this issue has landed:
Issue 7236 - Handle sub properties in abort-on-property snippets

comment:12 Changed 2 days ago by hfiguiere

  • Description modified (diff)

comment:13 Changed 2 days ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.