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

[java] use FileDetector to install Firefox addons #9872

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

titusfortner
Copy link
Member

@titusfortner titusfortner commented Sep 28, 2021

I don't see an ideal way to do this.

The whole point is to be able to do this with the Augmenter, which means the interfaces which don't know about RemoteWebDriver methods, and I don't want to put the driver instance as a parameter for the method in the interface.

This is the best I came up with.:

The implementation is mostly copied from RemoteWebElement#upload but again, I don't see an easy way to put this logic where both of them can access it.

Edit:
I changed this so it tries the string as provided first, and if that doesn't work, it determines if the file exists locally, and if so uploads it and retries.

Not sure it's ideal either, but it won't break anything, and won't keep us from doing something better later.

@@ -269,6 +269,13 @@ public String installExtension(Path path) {
return extensions.installExtension(path);
}

@Override
public String installExtension(Path path, FileDetector detector) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users really shouldn't need to know about the presence of the file detector.

INSTALL_EXTENSION,
ImmutableMap.of("path", path.toAbsolutePath().toString(), "temporary", false));
} catch (WebDriverException ex) {
File localFile = new LocalFileDetector().getLocalFile(path.toString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more correct thing to do would be to make RemoteExecuteMethod implement WrapsDriver. You could then get the driver, check if it's remote, and then get the file detector that way. Relying on exceptions for flow control is generally frowned upon.

@shs96c shs96c merged commit 5f1a0f8 into SeleniumHQ:trunk Sep 29, 2021
@sonarcloud
Copy link

sonarcloud bot commented Sep 29, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@titusfortner titusfortner deleted the java_addon_fd branch December 4, 2022 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants