Implement activeTab extension permission functionality on GeckoView
Categories
(GeckoView :: Extensions, defect, P1)
Tracking
(Not tracked)
People
(Reporter: robwu, Assigned: agi, Mentored)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression, Whiteboard: [geckoview:m82][geckoview:m85][geckoview:m87])
Attachments
(2 files, 2 obsolete files)
The unit test for the activeTab permission on mobile was removed in bug 1530402, and now we don't have any test coverage for this important functionality any more. It's not just the lack of test coverage, when I run a manual test, it is broken.
STR:
- Download the attached extension, unpack the attached extension to a directory.
- Start an Android emulator or attach a real Android device, with Fenix installed.
- Run
adb devices
to find the device ID (alternatively: run the command at the next step without the--android-device=...
flag to letweb-ext
find and show ID) - Use
web-ext run -t firefox-android --firefox-apk=org.mozilla.fenix --android-device=[ID from step 3 here]
(if you haven't done so, installweb-ext
first) - Visit example.com
- Click on the triple-dot menu and click on the "pageac titl" menu item.
Expected:
- A modal dialog showing "Dialog in http://example.com/"
Actual:
- Nothing happens.
- When I open
about:debugging
on desktop Firefox, click enable devices, attach to Firefox Preview, and click on the "Inspect" button at the "activeTab + pageAction" extension, then I can view the console for the extension, with the following error:
Going to executeScript in 10006 undefined
Failed to executeScript: Missing host permission for the tab
The error indicates that the activeTab permission was not activated when the pageAction button was clicked.
References:
- Bug where it was implemented in Fennec: bug 1389911
- Unit test before its removal: https://hg.mozilla.org/mozilla-central/diff/64fde5f3d49f19c9ab0d5e788f69ebc32b7d6a3f/mobile/android/components/extensions/test/mochitest/test_ext_activeTab_permission.html
Reporter | ||
Comment 1•4 years ago
|
||
Agi, did you deliberately remove this feature? Do you intend to restore this anytime soon?
Philipp, where does this stand on the list of priorities of APIs for GeckoView?
(note: the lack of activeTab permission support prevents extensions from using minimal permissions, which is opposing the goals of manifest v3)
Assignee | ||
Comment 2•4 years ago
|
||
Not intentional. We can look into it but unless one of the addons that we want to support needs this, it's not going to be high priority. We don't allow arbitrary addons yet anyway.
Assignee | ||
Comment 3•4 years ago
|
||
Looks like all we need to do is call this method on mobile too (or move that line to toolkit if possible) https://searchfox.org/mozilla-central/rev/4ccefc3181f9d237ef4ca8bd17b4e7c101ddf7b5/browser/components/extensions/parent/ext-browserAction.js#226
Assignee | ||
Comment 4•4 years ago
|
||
OK so steps to do in this bug:
-
Unconditionally send the
:Click
down to Gecko here: https://searchfox.org/mozilla-central/rev/4ccefc3181f9d237ef4ca8bd17b4e7c101ddf7b5/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebExtension.java#1374-1375 -
Call
this.tabManager.addActiveTabPermission();
when the click event is received here: https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewWebExtension.jsm#758-764 -
Don't call
browserAction.click()
when a popup is defined since now we always send the event down to Gecko -
Rewrite the test to not use Fennec classes here (probably like 90% of the work): https://hg.mozilla.org/mozilla-central/diff/64fde5f3d49f19c9ab0d5e788f69ebc32b7d6a3f/mobile/android/components/extensions/test/mochitest/test_ext_activeTab_permission.html
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
oops mixed up milestones
Probably the reason why my extension is broken on the new Firefox Android Nightly:
https://addons.mozilla.org/nl/firefox/addon/sitesearch/
When I change the permission from activeTab to tabs it starts working, but I'm not not inclined to request for additional permissions just because of bugs.
BTW stumbled on this bug several months ago, but never took the effort to track it down: IMHO testing for the new Android version isn't that attractive when there is NO way to install/test signed but unlisted extensions.
Should be higher on the priority list IMHO.
Reporter | ||
Comment 7•4 years ago
|
||
This is already high on the priority list (P2 is just below top priority), but we have a lot to work on. This is one of the bugs that should be fixed before add-ons become generally available on release.
web-ext
can be used to temporarily install an add-on for development (more info at https://extensionworkshop.com/documentation/develop/developing-extensions-for-firefox-for-android/ ). Contributors who want to try to work on a patch to resolve this bug can use this tool to manually test add-ons while working on a patch.
We have recently added support for installing any add-on on Nightly. But unlisted add-ons are not supported yet, as you might already have discovered.
Assignee | ||
Comment 8•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
oops. I wrote a patch a long time ago and forgot to submit it 😅 sorry about that.
Comment 10•4 years ago
|
||
Glad a patch is coming :)
Sure I used web-ext (nice tool) to find the bug, but I'm afraid I stick to the old Android version for daily use until my (precious) extensions do work, although I'm quite sure I will like the new version once that is the case.
Assignee | ||
Comment 11•4 years ago
|
||
This method allows callers to synchronously map a GeckoResult value to another
value.
Assignee | ||
Comment 12•4 years ago
|
||
This patch adds queryString
, queryBoolean
, queryVoid
and queryBundle
to
EventDispatcher
in place of dispatch(type, message, callback)
.
These query*
methods returns a GeckoResult
which can be manipulated using
GeckoResult.map
.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Comment on attachment 9187560 [details]
Bug 1625593 - Add GeckoResult.map.
Revision D96950 was moved to bug 1682668. Setting attachment 9187560 [details] to obsolete.
Assignee | ||
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Comment on attachment 9187561 [details]
Bug 1625593 - Use GeckoResult in EventDispatcher.
Revision D96951 was moved to bug 1685614. Setting attachment 9187561 [details] to obsolete.
Comment 15•4 years ago
|
||
Pushed by asferro@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/100bc29bbb72 Move activeTab functionality to toolkit. r=robwu,esawin
Comment 16•4 years ago
|
||
Backed out changeset 100bc29bbb72 (bug 1625593) for Browser-chrome failures in gfx/layers/apz/test/mochitest/browser_test_scrollbar_in_extension_popup_window.js. CLOSED TREE
Log:
https://treeherder.mozilla.org/logviewer?job_id=328256773&repo=autoland&lineNumber=2821
Push that shows failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=4572c1ba670a3ddb159cf37ba9684fb6055f260e
Backout:
https://hg.mozilla.org/integration/autoland/rev/2b57de6eddafb29c4ef30fc2a3ec66c0b5a2b185
Comment 17•4 years ago
|
||
Pushed by asferro@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f25813fc068d Move activeTab functionality to toolkit. r=robwu,esawin
Updated•4 years ago
|
Assignee | ||
Comment 18•4 years ago
|
||
That was a doozy. Thanks!
Comment 19•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Description
•