Opened 12 months ago

Closed 11 months ago

Last modified 9 months ago

#7141 closed change (fixed)

Implement abort-current-inline-script snippet — at Version 24

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

https://codereview.adblockplus.org/29953560/

Description (last modified by amrmak)

Background

Implement abort-current-inline-script snippet that will abort an inline script based on an API it accesses and the text content of the script.

While this looks like issue #6983, this snippet runs on Chrome as well.

What to change

Implement a snippet with the following signature:

function abortCurrentInlineScript(api, search = null)

The value of the api parameter may be a string like "window.location.href" or "console.log". If the specified API exists, it should be wrapped such that accessing the API causes the parent script to abort if it is an inline script.

The search parameter may be a string, optionally surrounded by / (slashes) so it is treated as a regular expression. If it is specified, the inline script should be aborted only if it contains the given pattern.

Note for testers

Test 1:

Serve the attached HTML document Test 1 over HTTP(S)

And add the following filter

example.com#$#abort-current-inline-script console.log Hello

(change the domain appropriately)

The console should only have "There can only be one!" printed.

Note: the test might fail if the inline scripts are run before the snippet has a chance to run. The sample document works here, but this is not a guarantee that it is enough to run elsewhere.

Change History (25)

comment:1 Changed 12 months ago by hfiguiere

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

comment:2 Changed 12 months ago by hfiguiere

  • Status changed from new to reviewing

comment:3 Changed 12 months ago by hfiguiere

  • Description modified (diff)

comment:4 Changed 12 months ago by mjethani

We could add one more example here:

<!DOCTYPE html>
<body>
 <script>
 console.log("Hello from inline script!");
 </script>
 <script>
 console.log("Hello from inline JavaScript!");
 </script>
 <script>
 console.log("There can only be one!");
 </script>
</body>

And the filter:

example.com#$#abort-current-inline-script console.log '/inline [A-Za-z]*[Ss]cript/'
Last edited 12 months ago by mjethani (previous) (diff)

comment:5 Changed 12 months ago by hfiguiere

  • Description modified (diff)

comment:6 Changed 12 months ago by mjethani

  • Description modified (diff)

comment:7 Changed 12 months ago by mjethani

  • Description modified (diff)

Changed 11 months ago by hfiguiere

Test 1

comment:8 Changed 11 months ago by hfiguiere

  • Description modified (diff)

comment:9 Changed 11 months ago by abpbot

A commit referencing this issue has landed:
Issue 7141 - Implement abort-current-inline-script

comment:10 Changed 11 months ago by hfiguiere

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:11 Changed 11 months ago by mjethani

  • Priority changed from Unknown to P2
  • Ready set

comment:12 Changed 11 months ago by abpbot

A commit referencing this issue has landed:
Issue 7141 - Implement abort-current-inline-script

comment:13 Changed 11 months ago by mjethani

Note: the test might fail if the inline scripts are run before the snippet has a chance to run

Maybe we should add an explicit timeout instead to make the test more reliable? I have done this for other snippets (#7207 also has a timeout).

comment:14 Changed 11 months ago by mjethani

  • Description modified (diff)

comment:15 Changed 11 months ago by Ross

Working in the browser versions below. Will verify once able to test on minimum versions.

ABP 3.4.2.2231
Chrome 71.0.3578.98 / Windows 10
Firefox 64.0.2 / Windows 10

comment:16 Changed 11 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Implemented and working. Regex works fine.

ABP 3.4.2.2231
Chrome 71.0.3578.98 / Windows 10 (Latest)
Chrome 49.0.2623.75 / Windows 10 (Min supported)
Firefox 64.0.2 / Windows 10 (Latest)
Firefox 51.0 / Windows 10 (Min supported)
Opera 57.0.3098.76 / Windows 10 (Latest)
Opera 36.0.2130.65 / Windows 10 (Min supported)

comment:17 Changed 11 months ago by mjethani

  • Sensitive unset

comment:18 Changed 11 months ago by Ross

I have a question about this snippet after playing with it while creating test pages: What counts as an "api" here? Should I be able to define a function in a script tag on the page and target that as well as a function/api provided by the browser?

comment:19 Changed 11 months ago by mjethani

Yes, it's basically any global property, not necessarily a browser API.

comment:20 Changed 11 months ago by Ross

Thanks Manish.

comment:21 Changed 11 months ago by Ross

Does not work in Edge. Should it? I will open a new ticket if so.

ABP 0.9.12.2237
Edge 44.17763.1.0 / EdgeHTML 18.17763 / Windows 10

comment:22 Changed 10 months ago by mjethani

Let's open a new ticket for Edge, in my opinion.

comment:23 Changed 10 months ago by hfiguiere

It's possible that it is just a timing issue, as usual. This one is quite sensitive.

Yes please file a bug if you can't make it work on Edge.

comment:24 Changed 9 months ago by amrmak

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