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/ |
Description
Environment
Nexus One (emulator with API level 25)
Firefox Nightly 56.0a1
Adblock Plus 1.13.2.1784 (gecko-webext)
Steps to reproduce
- Open WebIDE to inspect open instances of Firefox Mobile
- Select "Main Process"
- Switch to "Console" tab
- 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)
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
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.
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: ↓ 14 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.
- The error is thrown for each navigation to a web page (independent of in which tab).
- The background page doesn't even receive the request from the content script.
- 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: ↓ 15 ↓ 19 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.
- 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.
- 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.
- 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.
comment:15 in reply to: ↑ 14 Changed on 08/08/2017 at 08:19:14 AM by mjethani
Replying to mjethani:
Replying to greiner:
- 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
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: ↓ 35 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:
- chrome.windows: #29516569
- chrome.contextMenus: #29511561
- chrome.browserAction: #29509573
Here are things we need to address for which I'm looking for your inputs:
- 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
- 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).
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
A commit referencing this issue has landed:
Issue 5347 - Open options page on Firefox for Android when browser action is clicked
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
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
The second error looks like it's the same underlying issue as #5386, but I'll try to confirm.