Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

As a Firefox extension developer, I would like to install a temporary extension from a folder #8357

Closed
seanpoulter opened this issue May 28, 2020 · 14 comments
Assignees

Comments

@seanpoulter
Copy link
Contributor

seanpoulter commented May 28, 2020

Edit: 2020-06-11 – ⚠️ This initial comment is not correct. I misunderstood the GeckoDriver API. I was looking at stale source code. 😓 With new knowledge and some guidance from the GeckoDriver team, I've proposed an approach to handle the "install addon" command from FirefoxDriver consistently for each language. See my long comment below.

--

🐛 Bug Report

We can't install a temporary extension in Firefox using the JavaScript or DotNet bindings. The implementation is inconsistent across the languages. The GeckoDriver API expects a path as a String but we're sending an addon. See mozilla/geckodriver - marionette.rs.

These two bindings need to be fixed:

Language Implementation Authors / CC
JavaScript javascript/node/selenium-webdriver/firefox.js#L637-L657 @kvetko, @jleyba
DotNet dotnet/src/webdriver/Firefox/FirefoxDriver.cs#L213-L227 @jimevans

These seem to be OK:

Language Implementation
Java java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java#L235-L240
Python py/selenium/webdriver/firefox/webdriver.py#L247-L264
Ruby rb/lib/selenium/webdriver/firefox/bridge.rb#L34-L38

To Reproduce

Try to install an extension without building an .xpi or .zip file using dotnet and javascript bindings.

TODO - Script

Expected behavior

I expect to be able to install a temporary extension in Firefox:

  • From the JavaScript library using installAddon
  • From the DotNet library using InstallAddOn
    • Note 1: This method is not in those docs?!
    • Note 2: InstallAddOnFromFile is also made redundant

Test script or set of commands reproducing this issue

TODO - Include in example repo.

The steps to reproduce are:

  • Create a minimal extension
    • extension\manifest.json
      {
        "name": "Uh oh!",
        "description" : "Print a message to the console you may not see",
        "version": "0.1",
        "manifest_version": 2,
        "content_scripts": [
          {
            "matches": ["https://*.github.com/*"],
            "js": ["index.js"]
          }
        ]
      }
    • extension\index.js
      console.log('We did it! ⭐️');
  • Start Firefox and add an extension
    • javascript/example.js
      const { Builder, By } = require('selenium-webdriver');
      const { Options } = require('selenium-webdriver/firefox');
      
      (async () => {
      
        const options =
          new Options()
            .setPreference('extensions.htmlaboutaddons.recommendations.enabled', false);
      
        const builder =
          new Builder()
          .forBrowser('firefox')
          .setFirefoxOptions(options);
      
        const driver = builder.build();
      
        const isTemporaryAddon = true;
        driver.installAddon('../extension/', isTemporaryAddon);
      
        await driver.get('https://github.com/SeleniumHQ/selenium');
      
        // Expected: A message to be printed on the console
        // Expected: The temporary extension appears in about:debugging > This Firefox > Temporary Extensions
      
      })();
  • Install selenium-webdriver
    echo {} > package.json
    npm install --save-dev selenium-webdriver
  • Run
    node javascript/example.js

Environment

OS: macOS
Browser: Firefox
Browser version: 71.0
Browser Driver version: GeckoDriver for the last 2+ years
Language Bindings version: Latest
Selenium Grid version (if applicable): Not applicable

Questions

  1. Would you welcome a PR?
  2. Do you want the issue or PR split by language?
@AutomatedTester
Copy link
Member

Please can we have a 2 separate PRs for each language. I will try get them merged ASAP

@seanpoulter
Copy link
Contributor Author

Will do. Thanks for the quick reply David.

@seanpoulter
Copy link
Contributor Author

seanpoulter commented Jun 12, 2020

Hey @AutomatedTester, before I submit any code changes I wanted to check-in with a bit of a pivot. I've had a good chat with some of the Mozilla devs on chat.mozilla.org from the interop and Add-ons channels.

Background

FirefoxDriver will dispatch a command to install an addon. GeckoDriver supports two variants of this command:

  • { addon: string, temporary: Optional }
  • { path: string, temporary: Optional

The "addon" is the Base64 encoded contents of a .xpi or .zip packed addon.
The "path" is a file path to a .xpi or .zip file. It can also be the path to an unpacked directory when:

  • The temporary is set
  • Firefox is configured to allow temporary extensions, either by changing the configuration or using the nightly or dev builds

What did I get wrong in the issue description?

  • GeckoDriver still supports both types of "install addon" command messages. I was looking at stale source code. See AddonInstallParameters in https://searchfox.org/mozilla-central/source/testing/geckodriver/src/command.rs.
  • We don't want to move away from "addon" at all. @whimboo has explained that when we're handling .xpi or .zip files, it is best to use the "addon" message. This allows GeckoDriver and the browser to be on separate machines. Otherwise, we can use the "path" message.

Proposed Changes

We'll want every language to use the same behaviour. When asked to install an addon, we should check the file type of the argument. When we have an .xpi or .zip file, we should send the command using the "addon"-type message. Otherwise, we'll use the "path".

To save space in the table below, I'll call it "🍍".

Language Current Behaviour Proposed Changes
.NET See FirefoxDriver.cs
  • InstallAddOnFromFile(path)
  • InstallAddOn(base64encodedString)
  • Change the signature of InstallAddOnFromFile to include the optional temporary flag
  • Update InstallAddOnFromFile. 🍍
  • Breaking change: Ideally we would rename InstallAddOnFromFile to InstallAddOn
Java See FirefoxDriver.java
  • installExtension(Path path)
  • Deprecate installExtension or remove in favour of installAddon.
  • Add installAddon(Path path) and installAddon(Path path, boolean temporary) methods.
  • 🍍
JavaScript See firefox.js
  • installAddon(path, temporary)
  • 🍍
Python See firefox/webdriver.py
  • install_addon(self, path, temporary=None)
  • 🍍
Ruby See firefox/bridge.rb
  • install_addon(path, temporary)
  • 🍍

We can also update the API docs to include a note that Firefox needs to be configured to allow temporary extensions.

--

Actions

@nickgaya
Copy link
Contributor

I'd like to suggest a different approach. Instead of checking the file extension or type, the API could allow the caller to specify whether the given path is local or remote. If local, then the code should read the file and send it to the server using the addon parameter. If remote, the code should send the path to the server using the path parameter.

I think this would be more flexible and give the caller more control over the behavior.

@hheexx
Copy link

hheexx commented Aug 26, 2021

Without temporary flag we can't add unsigned extensions

@bonigarcia
Copy link
Member

Is there any update about this issue? Thanks.

@titusfortner
Copy link
Member

Java fix is here - 932cbf9
I have a PR for .NET #10093

I'm working to get everything implemented with addon instead of path for Selenium 4.2 so we don't have to duplicate Local File Detector implementations for using with Remote drivers. There's not much downside to just encoding it even when on a local machine, even if geckodriver doesn't need it. JavaScript & .NET were already doing this, but Java support and Ruby support are in trunk, and I have a PR for Python: #10092

@titusfortner titusfortner added this to the 4.2 milestone Dec 3, 2021
@seanpoulter
Copy link
Contributor Author

That makes sense with addon and both PRs look great. Thank you Titus!

@bonigarcia
Copy link
Member

As the issue title says, I aim to install a temporary extension from a folder using Firefox. Using the Java bindings, this was already possible in Chrome using --load-extension as follows:

Path extension = Paths
        .get(ClassLoader.getSystemResource("web-extension").toURI());
ChromeOptions options = new ChromeOptions();
options.addArguments(
        "--load-extension=" + extension.toAbsolutePath().toString());
ChromeDriver driver = new ChromeDriver(options);

According to the release notes, this is also possible with Firefox in version 4.1.1, recently released. I tried to use the new feature in one of my tests as follows:

Path extension = Paths
        .get(ClassLoader.getSystemResource("web-extension").toURI());
FirefoxDriver driver = new FirefoxDriver();
driver.installExtension(extension, true);

But I get the following exception:

org.openqa.selenium.InvalidArgumentException: ...\target\test-classes\web-extension is an invalid path
Build info: version: '4.1.1', revision: 'e8fcc2cecf'
System info: host: 'DESKTOP-VPVQ23J', ip: '192.168.56.1', os.name: 'Windows 10', os.arch: 'amd64', os.version: '10.0', java.version: '1.8.0_251'
Driver info: driver.version: FirefoxDriver
	at org.openqa.selenium.firefox.AddHasExtensions$1.installExtension(AddHasExtensions.java:82)
	at org.openqa.selenium.firefox.FirefoxDriver.installExtension(FirefoxDriver.java:276)
	at io.github.bonigarcia.webdriver.jupiter.ch05.caps.extensions.LoadExtensionFirefoxJupiterTest.setup(LoadExtensionFirefoxJupiterTest.java:46)
...

Caused by: java.nio.file.AccessDeniedException: ...\test-classes\web-extension
	at sun.nio.fs.WindowsException.translateToIOException(Unknown Source)
	at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
	at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
	at sun.nio.fs.WindowsFileSystemProvider.newByteChannel(Unknown Source)
	at java.nio.file.Files.newByteChannel(Unknown Source)
	at java.nio.file.Files.newByteChannel(Unknown Source)
	at java.nio.file.Files.readAllBytes(Unknown Source)
	at org.openqa.selenium.firefox.AddHasExtensions$1.installExtension(AddHasExtensions.java:80)
	... 63 more

It seems that it is not possible to give the folder in which the extension (under development) is located. If I use the packaged extension (as a zip), I managed to make it work, but I would like to use the unpackaged folder.

Is it not possible to use an unpackaged extension in Firefox (like in Chrome with --load-extension)?

@titusfortner
Copy link
Member

First, in Chrome it would be easier to use the provided extension key instead of an argument, which is accessed via addExtensions()

Right now, all the languages are just working with zipped files not directories.
4.1.1 works if you zipped the directory yourself and used that file as the argument.

I think an argument can be made that this should be Selenium's responsibility instead of the user's responsibility, but since this isn't something that blocks a user from accessing the temporary extension functionality, I think it belongs in a separate feature request.

@bonigarcia
Copy link
Member

bonigarcia commented Apr 7, 2022

I think I found a problem in the current implementation of installExtension() for FirefoxDriver in Java. I use a method to zip the extension used in Firefox (see example). When the extension contains folders with assets (e.g., icons, JavaScript libraries, CSS, etc., for example this), these assets are not loaded by the extension in the browser controlled with Selenium.

@bonigarcia
Copy link
Member

I have just discovered that there was no problem in the implementation of installExtension() as I said in my previous post. The problem was in the method WebDriverManager.zipFolder() I used to compress the extension. Thanks to PR by @nickgaya, I managed to solve the issue, and now the assets within folders are correctly loaded in my tests. Thanks a lot, @nickgaya!

@titusfortner
Copy link
Member

This was merged, will be available in 4.5

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants