Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#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 2 years ago.
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 2 years ago by sebastian

  • Cc trev added

comment:2 Changed 2 years ago by saroyanm

  • Cc saroyanm added

comment:3 Changed 2 years ago by kzar

  • Cc kzar added

comment:4 Changed 2 years ago by kzar

  • Priority changed from Unknown to P3
  • Ready set

comment:5 Changed 2 years ago by greiner

  • Blocking 5384 added

comment:6 Changed 2 years ago by wspee

  • Cc wspee added

comment:7 Changed 2 years ago by sebastian

  • Cc mjethani added
  • Priority changed from P3 to P2

comment:8 Changed 2 years ago by mjethani

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

comment:9 Changed 2 years ago by mjethani

I'm able to reproduce this with Fifefox 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.

Version 0, edited 2 years ago by mjethani (next)

comment:10 Changed 2 years ago 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 2 years ago by mjethani

  • Owner set to mjethani

comment:12 Changed 2 years ago 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 2 years ago 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 2 years ago 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 2 years ago by mjethani (previous) (diff)

comment:15 in reply to: ↑ 14 Changed 2 years ago 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 2 years ago 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 2 years ago by mjethani

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

comment:17 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:18 Changed 2 years ago by mjethani

  • Status changed from new to reviewing

comment:19 in reply to: ↑ 14 Changed 2 years ago 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 2 years ago 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 2 years ago by mjethani (previous) (diff)

comment:21 Changed 2 years ago 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 2 years ago by mjethani

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

comment:23 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:24 Changed 2 years ago 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 2 years ago by mjethani (previous) (diff)

comment:25 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:26 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:27 Changed 2 years ago by abpbot

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

comment:28 Changed 2 years ago by abpbot

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

comment:30 Changed 2 years ago by abpbot

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

comment:31 Changed 2 years ago by abpbot

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

comment:32 Changed 2 years ago 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 2 years ago by mjethani

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

comment:34 Changed 2 years ago by trev

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

comment:35 in reply to: ↑ 22 Changed 2 years ago by mjethani

Replying to mjethani:

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

This is fixed now.

comment:36 Changed 2 years ago 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

Note: See TracTickets for help on using tickets.