Opened 16 months ago

Last modified 16 months ago

#6841 closed defect

Snippets do not work in Chrome 49 — at Version 11

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: Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

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

Description (last modified by kzar)

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)
  • If we instead omit the frameId if it's 0 it starts working again.

Change History (11)

comment:1 Changed 16 months ago 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 16 months ago 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 16 months ago by Ross

  • Description modified (diff)

It also works in Chrome 50.

comment:4 Changed 16 months ago 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 16 months ago 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 16 months ago 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 16 months ago 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 16 months ago 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 16 months ago 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 16 months ago 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 16 months ago 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...

Note: See TracTickets for help on using tickets.