Open Bug 1806135 Opened 2 years ago Updated 3 days ago

Enable installation and uninstallation of addons in GeckoView

Categories

(Remote Protocol :: Marionette, enhancement, P3)

All
Android
enhancement
Points:
3

Tracking

(Not tracked)

People

(Reporter: twisniewski, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [webdriver:m13])

Attachments

(1 file)

While adding support for Android testing for Firefox's webcompat addon, I noticed that I could not issue a POST to moz/install/addon to install it on a Geckoview Example instance (which does not have it bundled).

It seems we have two assertions for Fennec which no longer make sense for Geckoview: https://searchfox.org/mozilla-central/source/remote/marionette/driver.sys.mjs#2847,2863

Could we remove those assertions? I am able to install the addon successfully without them.

Component: Agent → Marionette
Product: Remote Protocol → Testing

Those mentioned assertions were in place for Fennec where we weren't able to install addons. With the move to GeckoView this restriction doesn't have to be kept in place anymore and could be removed.

When doing that we might also want to move our existing Marionette unit tests for installing and uninstalling addons to the Mozilla specific web-platform test folder at:

https://searchfox.org/mozilla-central/source/testing/web-platform/mozilla/tests/webdriver/

Thomas, how important is it for you to have the ability to install addons? Is any webcompat work blocked on it?

Flags: needinfo?(twisniewski)
Summary: GeckoDriver assertions prevent the (un)installation of addons on Geckoview → Support (un)installation of addons in Geckoview
Type: defect → enhancement
OS: Unspecified → Android
Hardware: Unspecified → All
Summary: Support (un)installation of addons in Geckoview → Enable installation and uninstallation of addons in GeckoView

Yes, my work in bug 1805826 is realistically blocked by this. Geckoview Example doesn't have system addons/features bundled with it, so I need to install the webcompat addon manually. Otherwise anyone adding tests will have to work with full Fenix builds, which is significant effort compared to patching this, from what I can see. I have no problem working on this patch, if it helps.

Flags: needinfo?(twisniewski)

I see. So it would totally make sense to have that restriction removed then. If you could work on that patch we would kinda appreciate. I can help with anything that is needed to get the appropriate webdriver web-platform-tests written. It shouldn't be too hard.

Actually, it turns out that I can also install the addon without this patch, by executing a script in the chrome context, so there's less pressure to get this done.

It's worth noting as well that there is another issue; any addon installed won't also get permission to run in private browsing mode. So we would probably want to either have an option for that as well, a new endpoint to grant an addon permissions, or something along those lines.

I'm attaching my proof-of-concept patch from before I realized that I could just install the addon executing a chrome context script, in case it helps (while we decide how to best handle all of this API-wise).

Flags: needinfo?(hskupin)

Thanks Thomas. We will discuss the private browsing approach in one of our next triage meetings. Thanks for providing the PoC.

Flags: needinfo?(hskupin)
Whiteboard: [webdriver:triage]

Oh, and one other thing to note is that right now, one must use abd (via ADBDeviceFactory in my case) to push the XPI file to the emulator/device, as the endpoint expects the file to be on the local device, not wherever GeckoDriver is actually running. Maybe it would be nice for the install endpoint to do that file-pushing itself?

Please note that the add-on installation command can also take a base64 encode string. By using that there is no need to copy the XPI to the device first. As such the creation of the tests under eg. /testing/web-platform/mozilla/tests/webdriver/install_addon should be simple.

Will you take a try?

I don't mind, but would it not be best to test both cases? It's not difficult to transfer the file over using the device factory, even if only for the tests. That way we probably just need to agree on the details of how to enable addons in private browsing mode (a second API call, or a flag on the install call, etc). I don't actually know what the better approach is there, given the MV3 addon effort might end up making it necessary/useful to toggle other such permissions at some point.

We discussed the private browsing part in our triage meeting today. It's ok to get it added given that it is a vendor specific end-point. But how important is that feature to you? Is it crucial for your work? If yes and it needs to be also available through geckodriver then it will be a bit more work including Rust code changes. But I'm happy to help with whatever comes up.

(In reply to Thomas Wisniewski [:twisniewski] from comment #8)

I don't mind, but would it not be best to test both cases? It's not difficult to transfer the file over using the device factory, even if only for the tests.

Sadly we don't have that possibility for web-platform (wdspec) tests yet. Or maybe I miss something?

That way we probably just need to agree on the details of how to enable addons in private browsing mode (a second API call, or a flag on the install call, etc). I don't actually know what the better approach is there, given the MV3 addon effort might end up making it necessary/useful to toggle other such permissions at some point.

We actually want to re-use the existing API's and simply add a new flag to indicate the permissions. That way it will also be easy to extend for other possible flags in the future.

Whiteboard: [webdriver:triage]

Tom, we discussed this bug again in our triage meeting and we actually propose to split the work up into three bugs:

  1. Remove the asserts and add generic test cases to the mozilla specific wdspec directory for installing addons via a base64 encoded string (this bug).

  2. Lets file a follow-up bug to support Private Browsing.

  3. Lets file a follow-up bug to add tests for installing the addon from a path on the actual device.

Would that work for you? As you said above only 1) is blocking you at the moment. 2) will introduce quite a good amount of work including for geckodriver which we don't think should hold back landing 1).

Priority: -- → P3

Sure, I'm not actually blocked on this anymore, so I'm alright with however we'd like to split up the work.

I've filed two follow-up bugs which are bug 1810718 and bug 1810722.

Hi Tom. I wanted to ask if you are still interested to provide the patch for a minimal solution. If you don't have the time please let us know. Thanks.

Flags: needinfo?(twisniewski)

I currently don't have the time to finish my attached patch, so if someone else wants to beat me to it, please feel free. (I'm hoping to get it done over the next few weeks, but it's a low-priority task for me).

Flags: needinfo?(twisniewski)
Product: Testing → Remote Protocol
Points: --- → 3
Whiteboard: [webdriver:m13]
No longer blocks: 1810722
Duplicate of this bug: 1810722
Blocks: 1810722
No longer duplicate of this bug: 1810722
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: