Opened on 06/23/2017 at 04:57:15 PM

Closed on 08/24/2017 at 12:45:09 PM

Last modified on 09/14/2017 at 11:51:24 AM

#5347 closed defect (fixed)

[Web Extension] Errors thrown when opening webpage in Firefox Mobile

Reported by: greiner Assignee: mjethani
Priority: P2 Milestone: Adblock-Plus-1.13.4-for-Chrome-Opera
Module: Platform Keywords:
Cc: sebastian, trev, saroyanm, kzar, wspee, mjethani Blocked By:
Blocking: #5384 Platform: Firefox Mobile
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29509573/
https://codereview.adblockplus.org/29511561/
https://codereview.adblockplus.org/29516569/
https://codereview.adblockplus.org/29516656/
https://codereview.adblockplus.org/29516679/
https://codereview.adblockplus.org/29516684/

Description

Environment

Nexus One (emulator with API level 25)
Firefox Nightly 56.0a1
Adblock Plus 1.13.2.1784 (gecko-webext)

Steps to reproduce

  1. Open WebIDE to inspect open instances of Firefox Mobile
  2. Select "Main Process"
  3. Switch to "Console" tab
  4. On device: Go to example.com

Observed behavior

The following errors are shown:

can't access lexical declaration `framesOfTabs' before initialization  background.js:159
  createFrame moz-extension://53564d5a-48d8-4a86-a7a7-c1fa3d2f49ee/ext/background.js:159:9
  updatePageFrameStructure moz-extension://53564d5a-48d8-4a86-a7a7-c1fa3d2f49ee/ext/background.js:198:17
  <anonymous> moz-extension://53564d5a-48d8-4a86-a7a7-c1fa3d2f49ee/ext/background.js:278:5
  runSafeSyncWithoutClone resource://gre/modules/ExtensionUtils.jsm:52:14
  runSafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:176:38
  receiveMessage resource://gre/modules/ExtensionChild.jsm:763:61
  _callHandlers/< resource://gre/modules/MessageChannel.jsm:585:17
  _callHandlers resource://gre/modules/MessageChannel.jsm:584:14
  _handleMessage/deferred.promise< resource://gre/modules/MessageChannel.jsm:649:7
  _handleMessage resource://gre/modules/MessageChannel.jsm:646:24
  _handleMessage self-hosted:945:17
  receiveMessage resource://gre/modules/MessageChannel.jsm:163:5
response is undefined  include.preload.js:473
  apply/< moz-extension://53564d5a-48d8-4a86-a7a7-c1fa3d2f49ee/include.preload.js:473:1
  runSafeSyncWithoutClone resource://gre/modules/ExtensionUtils.jsm:52:14
  runSafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:176:38
  runSafeWithoutClone self-hosted:945:17
  wrapPromise/< resource://gre/modules/ExtensionCommon.jsm:355:13

Expected behavior

No errors are shown.

Further information

The cause for the latter error (i.e. ext.backgroundPage.sendMessage() calls callback without a value) also breaks the messaging between the extension's background page and its (upcoming) UI.

Attachments (1)

apis.sh (453 bytes) - added by mjethani on 08/08/2017 at 10:58:17 AM.
A shell script to print out the names of the chrome.* APIs used by Adblock Plus

Download all attachments as: .zip

Change History (37)

comment:1 Changed on 06/23/2017 at 05:00:48 PM by sebastian

  • Cc trev added

comment:2 Changed on 07/04/2017 at 10:15:22 AM by saroyanm

  • Cc saroyanm added

comment:3 Changed on 07/11/2017 at 04:13:59 PM by kzar

  • Cc kzar added

comment:4 Changed on 07/11/2017 at 04:15:30 PM by kzar

  • Priority changed from Unknown to P3
  • Ready set

comment:5 Changed on 07/13/2017 at 01:43:40 PM by greiner

  • Blocking 5384 added

comment:6 Changed on 08/01/2017 at 10:01:12 AM by wspee

  • Cc wspee added

comment:7 Changed on 08/01/2017 at 03:10:44 PM by sebastian

  • Cc mjethani added
  • Priority changed from P3 to P2

comment:8 Changed on 08/02/2017 at 12:17:51 PM by mjethani

The second error looks like it's the same underlying issue as #5386, but I'll try to confirm.

comment:9 Changed on 08/04/2017 at 12:54:27 PM by mjethani

I'm able to reproduce this with Firefox nightly on a Nexus One emulator API level 25 and one of the latest Adblock Plus development builds (1.13.3.1796).

I couldn't get WebIDE to connect to Firefox in the Android emulator, but I just did adb logcat | grep Gecko to view the errors.

Last edited on 08/04/2017 at 12:54:52 PM by mjethani

comment:10 Changed on 08/05/2017 at 05:16:02 PM by mjethani

It looks like it's going to be some work to get this working properly on Android. I managed to get rid of the first error, which was owing to the fact that chrome.windows isn't supported on Android. There were more similar errors that came up, I had to do feature/API detection for all of them.

Now the background page doesn't seem to be getting the message from the content script, which is the cause of the second error.

comment:11 Changed on 08/05/2017 at 05:16:55 PM by mjethani

  • Owner set to mjethani

comment:12 Changed on 08/07/2017 at 03:13:08 PM by mjethani

The second error is due to a bug in Firefox: sender.tab is undefined for the first tab. I've reported it: #1388066.

comment:13 follow-up: Changed on 08/07/2017 at 04:05:57 PM by greiner

Interesting issue that you found but it doesn't seem to correspond with what I've been experiencing.

  1. The error is thrown for each navigation to a web page (independent of in which tab).
  2. The background page doesn't even receive the request from the content script.
  3. The content script does get an affirmative reply but the message it receives is undefined. Presumably, the content script in your extension will log the error or nothing at all rather than "background.js says: undefined".

So is this difference caused by the way our messaging abstraction layer is implemented/used? For instance, we're using callbacks instead of Promises to react to replies from the background page.

comment:14 in reply to: ↑ 13 ; follow-ups: Changed on 08/08/2017 at 03:26:22 AM by mjethani

Replying to greiner:

Interesting issue that you found but it doesn't seem to correspond with what I've been experiencing.

  1. The error is thrown for each navigation to a web page (independent of in which tab).

You'll get the "response is undefined" error in the content script if the message handler in the background page throws an exception. It can throw an exception for any number of reasons. For example, framesOfTabs is not initialized, which happens because the code before the let statement threw an exception, which in turn happened because chrome.windows is undefined, and so on. Basically the initialization of the extension is broken, which then causes lots of errors during each page load.

The issue I've reported is very specific and it happens after many of the other errors have been fixed.

  1. The background page doesn't even receive the request from the content script.

After you fix the other errors, the background page does receive the message.

  1. The content script does get an affirmative reply but the message it receives is undefined. Presumably, the content script in your extension will log the error or nothing at all rather than "background.js says: undefined".

This is a feature of messaging in WebExtensions. If the message handler at the other end throws an exception, the response will be undefined.

So "response is undefined" is a symptom of the fact that we've got lots of errors.

So far I've run into the following objects being undefined on Firefox for Android: chrome.windows, chrome.contextMenus, and chrome.browserAction.setIcon. There are lots more. We'll have to comb through the code and either wrap these into if blocks or do something appropriate for Android.

Last edited on 08/08/2017 at 03:30:55 AM by mjethani

comment:15 in reply to: ↑ 14 Changed on 08/08/2017 at 08:19:14 AM by mjethani

Replying to mjethani:

Replying to greiner:

  1. The background page doesn't even receive the request from the content script.

After you fix the other errors, the background page does receive the message.

If you meant immediately after installation, then yes, there's a bug in Firefox because of which the content scripts are run before the background page has been loaded.

I've reported it here: #1388270.

comment:16 Changed on 08/08/2017 at 09:24:17 AM by mjethani

I ran grep -EwroIsh 'chrome(\.\w+)+' . | sort -u on the adblockpluschrome repo and this is what I got:

chrome.browserAction.set
chrome.browserAction.setBadgeBackgroundColor
chrome.browserAction.setBadgeText
chrome.browserAction.setIcon
chrome.contextMenus.create
chrome.contextMenus.removeAll
chrome.devtools
chrome.devtools.inspectedWindow.tabId
chrome.devtools.panels.create
chrome.devtools.panels.themeName
chrome.extension
chrome.extension.getBackgroundPage
chrome.extension.getURL
chrome.i18n
chrome.notifications.clear
chrome.notifications.create
chrome.notifications.onButtonClicked.addListener
chrome.notifications.onClicked.addListener
chrome.notifications.onClosed.addListener
chrome.runtime
chrome.runtime.connect
chrome.runtime.lastError
chrome.runtime.lastError.message
chrome.runtime.onConnect.addListener
chrome.runtime.onMessage.addListener
chrome.runtime.openOptionsPage
chrome.runtime.reload
chrome.runtime.sendMessage
chrome.runtime.setUninstallURL
chrome.storage
chrome.storage.local.get
chrome.storage.local.remove
chrome.storage.local.set
chrome.storage.managed
chrome.storage.managed.get
chrome.storage.onChanged
chrome.tabs.create
chrome.tabs.get
chrome.tabs.insertCSS
chrome.tabs.onActivated.addListener
chrome.tabs.onRemoved.addListener
chrome.tabs.onRemoved.removeListener
chrome.tabs.onReplaced.addListener
chrome.tabs.onReplaced.removeListener
chrome.tabs.onUpdated.addListener
chrome.tabs.onUpdated.removeListener
chrome.tabs.query
chrome.tabs.reload
chrome.tabs.remove
chrome.tabs.sendMessage
chrome.tabs.update
chrome.webNavigation
chrome.webNavigation.getAllFrames
chrome.webNavigation.onBeforeNavigate.addListener
chrome.webNavigation.onBeforeNavigate.removeListener
chrome.webNavigation.onCommitted.addListener
chrome.webNavigation.onCommitted.removeListener
chrome.webNavigation.onCompleted.addListener
chrome.webNavigation.onCompleted.removeListener
chrome.webNavigation.onCreatedNavigationTarget.addListener
chrome.webRequest.MAX_HANDLER_BEHAVIOR_CHANGED_CALLS_PER_10_MINUTES
chrome.webRequest.ResourceType
chrome.webRequest.handlerBehaviorChanged
chrome.webRequest.onBeforeRequest.addListener
chrome.webRequest.onBeforeRequest.removeListener
chrome.webRequest.onHeadersReceived.addListener
chrome.windows.WINDOW_ID_NONE
chrome.windows.create
chrome.windows.getLastFocused
chrome.windows.onFocusChanged.addListener
chrome.windows.update

For reference, in case anyone else is interested.

The task at hand is to figure out which of these APIs are not supported on Firefox for Android and deal with them accordingly.

Changed on 08/08/2017 at 10:58:17 AM by mjethani

A shell script to print out the names of the chrome.* APIs used by Adblock Plus

comment:17 Changed on 08/08/2017 at 11:47:18 AM by mjethani

  • Review URL(s) modified (diff)

comment:18 Changed on 08/08/2017 at 11:47:30 AM by mjethani

  • Status changed from new to reviewing

comment:19 in reply to: ↑ 14 Changed on 08/08/2017 at 11:52:27 AM by greiner

Replying to mjethani:

Basically the initialization of the extension is broken, which then causes lots of errors during each page load.
The issue I've reported is very specific and it happens after many of the other errors have been fixed.

Ok, gotcha, thanks for the detailed clarification.

comment:20 Changed on 08/12/2017 at 09:24:42 PM by mjethani

chrome.notifications on Firefox does not support the buttons property (see #1190681), nor does the Notifications API support the actions property (same underlying issue), nor is Window.confirm supported in background pages.

I'm not sure what we can do about this if they don't implement support.

For reference, we use these APIs here:

https://hg.adblockplus.org/adblockpluschrome/file/552ac1922795/lib/notificationHelper.js#l196
https://hg.adblockplus.org/adblockpluschrome/file/552ac1922795/lib/notificationHelper.js#l212
https://hg.adblockplus.org/adblockpluschrome/file/552ac1922795/lib/notificationHelper.js#l231

Last edited on 08/12/2017 at 09:32:24 PM by mjethani

comment:21 Changed on 08/13/2017 at 12:56:52 PM by mjethani

For desktop, our best bet might be to show the question notification in the popup and draw attention to it using the icon animation, since we already have a lot of the code for that in place. We could also use chrome.windows.create to open a new "panel", though it might be a bit more work.

comment:22 follow-up: Changed on 08/14/2017 at 07:51:47 PM by mjethani

runtime.openOptionsPage is not yet supported on Android either: #1364945.

comment:23 Changed on 08/15/2017 at 02:12:14 PM by mjethani

  • Review URL(s) modified (diff)

comment:24 Changed on 08/15/2017 at 02:45:25 PM by mjethani

Taking stock of this issue.

Here's a list of the changes in review so far to get this working on Android:

  1. chrome.windows: #29516569
  2. chrome.contextMenus: #29511561
  3. chrome.browserAction: #29509573

Here are things we need to address for which I'm looking for your inputs:

  1. chrome.notifications doesn't support the buttons option, but we could show a button-less notification and open a new tab (using chrome.tabs.create) on click
  2. chrome.runtime.openOptionsPage is not supported on Android, but somehow you can still call the function, it just doesn't do anything (which means we can't do feature/API detection); we'll have to resort to checking the platform here using the info module, and do the same thing we do on Edge (i.e. open the options page in a new tab)

All other APIs the extension uses are supported on Android, though there may be differences in behavior that we might run into.

For mobile, we also don't want to load files like composer.postload.js in the content script. I think gecko-mobile-webext should probably be a separate build target, but I'm not sure (looking for your inputs).

Last edited on 08/15/2017 at 02:46:20 PM by mjethani

comment:25 Changed on 08/15/2017 at 04:18:10 PM by mjethani

  • Review URL(s) modified (diff)

comment:26 Changed on 08/16/2017 at 08:26:14 PM by mjethani

  • Review URL(s) modified (diff)

comment:27 Changed on 08/17/2017 at 02:09:15 PM by abpbot

A commit referencing this issue has landed:
Issue 5347 - Check for contextMenus API support

comment:28 Changed on 08/18/2017 at 09:21:45 AM by abpbot

A commit referencing this issue has landed:
Issue 5347 - Check for browserAction API support

comment:29 Changed on 08/21/2017 at 02:03:47 PM by abpbot

comment:30 Changed on 08/24/2017 at 11:23:01 AM by abpbot

A commit referencing this issue has landed:
Issue 5347 - Do not show composer menu item on Firefox for Android

comment:31 Changed on 08/24/2017 at 11:27:07 AM by abpbot

A commit referencing this issue has landed:
Issue 5347 - Check for windows API support

comment:32 Changed on 08/24/2017 at 12:44:29 PM by mjethani

After all the changes, there are no longer any errors on Firefox for Android when a page is loaded.

There is still a "response is undefined" error in the content script when the extension is installed (on both desktop and mobile), but this is because Firefox runs the content script before the background page has been loaded. We could return from the elemhide.getSelectors handler if response is undefined, but it would have the same effect as the exception being thrown. We can safely ignore this error for now.

Since all the errors during page load are gone, I'm closing this.

comment:33 Changed on 08/24/2017 at 12:45:09 PM by mjethani

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:34 Changed on 08/24/2017 at 12:47:10 PM by trev

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next

comment:35 in reply to: ↑ 22 Changed on 08/25/2017 at 06:07:50 AM by mjethani

Replying to mjethani:

runtime.openOptionsPage is not yet supported on Android either: #1364945.

This is fixed now.

comment:36 Changed on 09/14/2017 at 11:51:24 AM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done as described. Errors/warnings no longer occur on Firefox Mobile when installing ABP, apart from the ones mentioned such as "response is undefined".

ABP 2.99.0.1838beta
Firefox Nightly 57.0a1 / Samsung J5

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 mjethani.
 
Note: See TracTickets for help on using tickets.