Opened 5 months ago

Closed 5 months ago

Last modified 4 months ago

#6782 closed change (fixed)

Implement basic support for snippets

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone: Adblock-Plus-3.3-for-Chrome-Opera-Firefox
Module: Platform Keywords: circumvention
Cc: kzar, sebastian, hfiguiere, arthur, jgarcia, amrmak, jsonesen Blocked By: #6781, #6784, #6841
Blocking: #6539 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29737561/

Description (last modified by mjethani)

Background

#6538 and #6539 describe the Snippets feature. #6781 describes what goes in the initial implementation in the Core module.

The initial implement of the Snippets feature in the Platform module comes with the following limitations:

  • The limitations described in #6781
  • Snippet hits will not be logged to the DevTools panel

What to change

Update the adblockpluscore dependency in the dependencies file to a version of core that includes support for snippet filters (#6784).

In metadata.chrome, include adblockpluscore/lib/content/snippets.js locally in the build as snippets.js.

Since we intend to reuse the same module for both element hiding and snippets, rename lib/cssInjection.js to lib/contentFiltering.js and update metadata.chrome accordingly.

In lib/contentFiltering.js:

  • Load the contents of ./snippets.js in source form using the fetch API and store the resulting text in a variable called snippetsLibrarySource
  • Import the Snippets and compileScript APIs from core's lib/snippets.js
  • Rename the elemhide.getSelectors message to content.applyFilters
  • In the listener for the content.applyFilters message, if the document is not whitelisted (only via the $document whitelisting option), look up the scripts for the document's domain using Snippets.getScriptsForDomain
  • For each script, call compileScript(script, [snippetsLibrarySource]) to get the executable JavaScript code
  • Inject the executable code into the document using the tabs.executeScript extension API with the options {matchAboutBlank: true, runAt: "document_start"}
  • Do any error handling and update the documentation as necessary and appropriate

In include.preload.js:

  • Rename ElemHide to ContentFiltering and elemhide to contentFiltering
  • Send the content.applyFilters message rather than the elemhide.getSelectors message to the background page

Make any changes to any other files as necessary for compatibility.

Hints for testers

Use the hints in #6781 as a start.

The changes in the Platform module also affect element hiding and element hiding emulation; therefore, make sure that these features work as they did before.

Since there is no DevTools logging in this version, the only way to test the application of a snippet filter on a document is by using the log snippet as described in #6781. It is understood and accepted that testing will be difficult given the limited output available in the DevTools console. This is an alpha version of the feature. It is more important to ensure that there are no regressions.

If there is a whitelisting filter with the $document option that applies to a given document, no snippet filters should be applied to that document. For example, if there is @@||example.com^$document then example.com#$#log 'Hello, world!' should not log "Hello, world!" to the console.

The syntax of snippet filters, as described in #6538, should be tested thoroughly. This can be done using the log snippet. For example, example.com#$#log 'What\'s up!? ¯\\_(\u30c4)_/' should correctly log "What's up!? ¯\_(ツ)_/" to the DevTools console (quoting and escaping, including Unicode escaping, work correctly).

Unicode character literals in a snippet filter, including high code points (beyond the 16-bit range), should get interpreted correctly. If a filter invokes the log snippet with a purple heart followed by a red heart, for example, this should correctly log a purple heart followed by a red heart to the console.

If a snippet filter contains a call to a non-existent snippet (like example.com#$#non-existent-snippet), it should fail gracefully (silently) and the rest of the snippet filters should be applied.

Test snippet scripts containing multiple snippet invocations. For example, example.com#$#log Hello; log 'How are you?' should call the log snippet twice; the console should show "Hello" and "How are you?", each on its own line, in that order.

Snippet names and arguments should be interpreted internally as strings, so there should be no possibility of arbitrary code execution. For example, example.com#$#log '(function(){window.alert("I steal your cookies!")})()' should not show an alert, it should just log the literal string to the console.

When new element hiding filters are added via the composer, only the element hiding filters should be applied, any snippet filters should not get applied again.

Run the tests mentioned in #6841 to make sure that snippets are being injected only into the right frames.

Change History (20)

comment:1 Changed 5 months ago by mjethani

  • Description modified (diff)

comment:2 Changed 5 months ago by hfiguiere

  • Cc hfiguiere added

comment:3 Changed 5 months ago by mjethani

  • Keywords circumvention added

comment:4 Changed 5 months ago by mjethani

  • Status changed from new to reviewing

comment:5 Changed 5 months ago by mjethani

  • Sensitive set

comment:6 Changed 5 months ago by mjethani

  • Blocked By 6784 added

comment:7 Changed 5 months ago by mjethani

  • Description modified (diff)

comment:8 Changed 5 months ago by kzar

A commit referencing this issue has landed:
Issue 6539, 6782 - Implement support for snippets

comment:9 Changed 5 months ago by kzar

Please could you flesh out this issue so I can triage? It's totally valid if there's just a brief introduction followed by links to the relevant other issues, I realise you've put a lot of the details elsewhere. I guess it would also be good to clarify what the "basic support" subset of the snippets feature includes.

comment:10 Changed 5 months ago by mjethani

Yes, I'll add all the details of the design to #6539, then I'll add here what's going to be missing in the initial version.

I intend to do this on Monday.

comment:11 Changed 5 months ago by mjethani

  • Description modified (diff)

comment:12 Changed 5 months ago by mjethani

  • Description modified (diff)

comment:13 Changed 5 months ago by mjethani

@kzar I have added enough information now, let me know if you think anything is missing.

comment:14 Changed 5 months ago by mjethani

  • Description modified (diff)

comment:15 Changed 5 months ago by mjethani

  • Cc arthur jgarcia amrmak jsonesen added

comment:16 Changed 5 months ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Priority changed from Unknown to P2
  • Ready set
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:17 Changed 4 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Basic support for snippets is implemented and all of the test cases listed above work in all browsers except Chrome 49 (#6841).

ABP 3.2.2102
Chrome 66 / 55 / Windows 10
Firefox 61 / 55 / 51 / Windows 10

comment:18 Changed 4 months ago by mjethani

  • Description modified (diff)

comment:19 Changed 4 months ago by mjethani

  • Blocked By 6841 added
  • Description modified (diff)

comment:20 Changed 4 months ago by sebastian

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