Opened on 08/07/2018 at 07:43:37 PM

Closed on 08/08/2018 at 03:13:54 PM

Last modified on 08/14/2018 at 11:40:23 AM

#6841 closed defect (fixed)

Snippets do not work in Chrome 49

Reported by: Ross Assignee: kzar
Priority: P1 Milestone: Adblock-Plus-3.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, kzar, mjethani, rscott Blocked By:
Blocking: #6782 Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

​https://codereview.adblockplus.org/29850574/

Description (last modified by mjethani)

Environment

ABP 3.2.2102
Chrome 49 / Windows 10

Could not reproduce in (snippets work fine):
Chrome 66 / 55 / 50 / Windows 10
Firefox 61 / 55 / 51 / Windows 10

How to reproduce

  1. Add snippet to custom filters adblockplus.org#$#log 'Hello, world!'
  2. Navigate to ​https://adblockplus.org

Observed behaviour

The snippet does not seem to run. "Hello, world!" is not printed to the console as in other browsers/versions.

Expected behaviour

Snippets to work in Chrome 49 (unless there is a reason they cannot).

Notes

  • adblockpluschrome/lib/contentFiltering.js is calling browser.tabs.executeScript, passing a frameId of 0 which causes an exception in Chrome 49 (which we silently ignore): 
browser.tabs.executeScript failed Error: Invalid value for argument 2. Property 'frameId': Unexpected property.
    at validate (extensions::schemaUtils:34:13)
    at Object.normalizeArgumentsAndValidate (extensions::schemaUtils:117:3)
    at Object.<anonymous> (extensions::binding:374:30)
    at Object.value [as executeScript] (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/polyfill.js:108:14)
    at executeScript (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:11490:18)
    at port.on (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:11537:9)
    at Object._onMessage (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:3758:22)
    at Object._dispatch (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/ext/common.js:41:22)
    at browser.runtime.onMessage.addListener (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/ext/background.js:604:26)
    at wrapper (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/polyfill.js:156:20)

Hints for testers

Given the filter 127.0.0.1#$#log 'Hello from the main frame' and the following document loaded off 127.0.0.1:

<html>
  <body>
    <h1>Main Frame</h1>
    <iframe src="http://example.com"></iframe>
  </body>
</html>

You should see only one instance of the text "Hello from the main frame" printed to the console. Likewise, changing the filter to example.com#$#log 'Hello from example.com' should print "Hello from example.com" only once. But changing the filter to 127.0.0.1,example.com#$#log 'Hello' should print "Hello" twice.

Make sure that snippets intended for a subframe are not injected into the top frame. To do this, use this document:

<html>
  <head>
    <script>window.console = null;</script>
  </head>
  <body>
    <h1>Main Frame</h1>
    <iframe src="http://example.com"></iframe>
  </body>
</html>

The filter example.com#$#trace 'Hello from example.com' should print "Hello from example.com"; if an error is printed to the console (because the inline script assigns the console object to null), it means the snippet is being injected into the top frame.

Please test this on both Chrome and Firefox.

Attachments (0)

Change History (22)

comment:1 Changed on 08/08/2018 at 09:38:17 AM by mjethani

Ross, can you add details about what filter lists you're subscribed to?

I am unable to reproduce this at my end.

comment:2 Changed on 08/08/2018 at 11:09:19 AM by Ross

I've tried with the full set of default filter lists just after an install, just with EasyList, with AA on and off.

The snippet adblockplus.org#$#log 'Hello, world!' prints to the console in Chrome 55, but does not for me in Chrome 49. I don't see any errors in any consoles. Does it print in 49 for you?

comment:3 Changed on 08/08/2018 at 11:42:26 AM by Ross

  • Description modified (diff)

It also works in Chrome 50.

comment:4 Changed on 08/08/2018 at 01:07:45 PM by mjethani

My bad, I was testing with a newer version of Chrome. I do see now that it doesn't work.

However the issue appears to be something else. I would be surprised if element hiding was working at all. I see this error in the background console:

extensions::uncaught_exception_handler:8 Error in response to tabs.get: Error: Invalid value for argument 1. Property 'path': Value does not match any valid type choices.
    at Object._applyChanges (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/ext/background.js:333:35)
    at Object.browser.tabs.get [as callback] (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/ext/background.js:388:16)
    at Object.value [as get] (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/polyfill.js:95:23)
    at Object._queueChanges (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/ext/background.js:368:20)
    at Object._addChange (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/ext/background.js:397:14)
    at Object.setIcon (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/ext/background.js:404:12)
    at setIcon (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/lib/adblockplus.js:10446:26)
    at FilterNotifier.on (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/lib/adblockplus.js:10463:5)
    at Object.emit (chrome-extension://bhignfpcigccnlfapldlodmhlidjaion/lib/adblockplus.js:4460:7)

comment:5 Changed on 08/08/2018 at 01:32:38 PM by Ross

Hmm, I don't see that error at all and the Element Hiding and Element Hiding Emulation filter test pages seem to pass as expected in 49 for me.

I'm using Chrome 49 x64 on Windows 10, not that that should make much of a difference.

comment:6 Changed on 08/08/2018 at 01:42:21 PM by mjethani

OK, there are multiple issues here.

First of all, at least on macOS 10.12.6, Chrome 49 does not support certain APIs. Secondly, there appears to be a bug in Chrome 49 with arrow functions.

As for the specific issue of snippets, I get this error (silently ignored in the development build):

Error: Invalid value for argument 2. Property 'frameId': Unexpected property.

It seems the frameId property was added in a later release. In that case, we cannot really run snippets reliably on Chrome 49 without messing something up. It's best to leave this as it is. i.e. "won't fix" (snippets are not supported on Chrome 49, at least for now).

comment:7 Changed on 08/08/2018 at 01:52:01 PM by mjethani

Ross, in chrome://extensions can you click on the background page link and see if there are any other errors in the console? Switch between tabs, load a page in the browser, and see if you're getting any errors. These might be separate issues, or maybe it's just me (my build of Chrome 49 may not be the final release build).

comment:8 Changed on 08/08/2018 at 02:07:34 PM by kzar

I can reproduce this, I can't see any exceptions. I can see that getScriptsForDomain is being called correctly and is returning the correct result. Continuing to investigate...

comment:9 Changed on 08/08/2018 at 02:17:48 PM by kzar

browser.tabs.executeScript is failing, I'm getting this exception but we silently ignore it:

browser.tabs.executeScript failed Error: Invalid value for argument 2. Property 'frameId': Unexpected property.
    at validate (extensions::schemaUtils:34:13)
    at Object.normalizeArgumentsAndValidate (extensions::schemaUtils:117:3)
    at Object.<anonymous> (extensions::binding:374:30)
    at Object.value [as executeScript] (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/polyfill.js:108:14)
    at executeScript (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:11490:18)
    at port.on (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:11537:9)
    at Object._onMessage (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/lib/adblockplus.js:3758:22)
    at Object._dispatch (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/ext/common.js:41:22)
    at browser.runtime.onMessage.addListener (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/ext/background.js:604:26)
    at wrapper (chrome-extension://ffiagampjdjojfgakjeeoipjmfmkahnn/polyfill.js:156:20)

comment:10 Changed on 08/08/2018 at 02:24:25 PM by kzar

It's since we're passing the frameId value to browser.tabs.executeScript. I suggest if frameId is 0 we just omit it. That seems to work. For other frames I guess we have to accept it won't work on Chrome 49. What do you think?

comment:11 Changed on 08/08/2018 at 02:37:09 PM by kzar

  • Component changed from Core to Platform
  • Description modified (diff)
  • Owner set to kzar
  • Priority changed from Unknown to P1
  • Ready set

Coming up with a patch for that...

comment:12 Changed on 08/08/2018 at 02:50:07 PM by kzar

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

comment:13 Changed on 08/08/2018 at 03:08:16 PM by kzar

  • Description modified (diff)

comment:14 Changed on 08/08/2018 at 03:13:03 PM by abpbot

A commit referencing this issue has landed:
​Issue 6841 - Avoid passing frameId of 0 to tabs.executeScript

comment:15 Changed on 08/08/2018 at 03:13:54 PM by kzar

  • Milestone set to Adblock-Plus-3.3-for-Chrome-Opera-Firefox
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:16 Changed on 08/08/2018 at 03:17:53 PM by kzar

  • Cc rscott added

Adding you as Cc Rick in case you missed this. We just pushed another hotfix for the release, sorry!

Last edited on 08/08/2018 at 03:19:51 PM by kzar

comment:17 Changed on 08/09/2018 at 05:30:26 AM by mjethani

  • Description modified (diff)

comment:18 Changed on 08/09/2018 at 11:07:43 AM by kzar

Thanks for adding those testing hints Manish, it will be interesting to see how Chrome 49 handles injecting snippets into frames. It's possible that won't work either, but there won't be much we can do there, and it'll fail silently at least.

comment:19 Changed on 08/09/2018 at 12:04:33 PM by mjethani

  • Description modified (diff)

comment:20 Changed on 08/09/2018 at 12:05:20 PM by mjethani

I've added some more testing instructions. We need to make sure snippets are not injected into the wrong frame.

comment:21 Changed on 08/09/2018 at 04:42:14 PM by mjethani

  • Blocking 6782 added

comment:22 Changed on 08/14/2018 at 11:40:23 AM by Ross

  • Verified working set

Snippets now work in Chrome 49. The test cases in the testing notes work as described.

However as kzar mentioned, snippets do not seem to work inside of frames. They do on Chrome 50+ and all Firefox versions tested for reference.

ABP 3.2.2103
Chrome 66 / 55 / 49 / Windows 10
Firefox 61 / 55 / 51 / Windows 19

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 kzar.
Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.