MachinePublishers / jBrowserDriver

A programmable, embeddable web browser driver compatible with the Selenium WebDriver spec -- headless, WebKit-based, pure Java
Other
809 stars 143 forks source link

1.0.2 release - To work with current release version of Selenium 3 (v3.141.59) #354

Closed PredatorVI closed 4 years ago

PredatorVI commented 4 years ago

Running against Oracle JDK 8u212 using JBrowserDriver v1.0.1 failed due to additional methods added to the com.sun.glass.ui.Timer abstract class. This was fixed already in #350. However, commit #347 changed the selenium version to v4.0.0-alpha-1 and added references to a WindowType class that is not available in Selenium 3.141.x.

This pull request fixes #353 to work with Selenium 3.141.x by merging all changes compatible with Selenium 3.141.x. I prefer not to use an alpha version of Selenium in my production environment, while needing to be able to use security and other updates that are part of the Oracle Java 8u212 and later, specifically Java 8u221

PredatorVI commented 4 years ago

I'm a n00b when it comes to pull requests, but this likely should be merged to a branch other than master and released as a new version (e.g., v1.0.2).

hollingsworthd commented 4 years ago

Sorry I wasn't able to connect the dots on the bug report you had. I reviewed the PR and think we can keep Selenium 3 support in addition to Selenium 4.

It would be a matter of commenting out the @Override annotations on TargetLocator.newWindow(WindowType) implementations and copying in org.openqa.selenium.WindowType from Selenium 4 into this project.

Then if we kept Selenium 4 in this project's dependencies users could decide to have Selenium 3 instead by declaring that dependency in their own project. I think that Maven's conflict resolution will work that way.

I've cherry-picked about half of the changes from your branch (the POM-related ones). Unfortunately my test machine no longer can run Java 8. So I can't test out what I've done until I get a VM set up.

Jfyi, Java 11 support is started here https://github.com/MachinePublishers/jBrowserDriver/tree/java11 but there are some showstoppers still. Note I plan to be rebasing that branch.

Let me know your thoughts. Thanks for taking time with this to look into the issue and submit this!

PredatorVI commented 4 years ago

Being slightly obsessive with keeping code more pristine, I'd look at a Java 8 + Se3 JBrowser branch (v1.0.x) with master branch (v1.1.x) being the latest and greatest using Se4, but I also realize that increases overhead and introduces issues with how to support all combinations of Java and Selenium.

I'm happy with whatever you feel is best. Ultimately, I'd simply like to be able to run latest Oracle JDK 8 with the latest stable release of selenium with a version of JBrowser that works in my continuous deployment environment (Ubuntu 16.04 headless). It's been a lifesaver for this specific workflow.

Let me know if you'd like me to help test any changes you make or want me to update the pull request in some way. As mentioned (I think), I'm still new to the process of doing pull requests/merging code, etc.

I'm also happy to test any builds you are working on.

On Wed, Sep 25, 2019 at 11:08 PM D. Hollingsworth notifications@github.com wrote:

Sorry I wasn't able to connect the dots on the bug report you had. I reviewed the PR and think we can keep Selenium 3 support in addition to Selenium 4.

It would be a matter of commenting out the @Override annotations on TargetLocator.newWindow(WindowType) implementations and copying in org.openqa.selenium.WindowType from Selenium 4 into this project.

Then if we kept Selenium 4 in this project's dependencies users could decide to have Selenium 3 instead by declaring that dependency in their own project. I think that Maven's conflict resolution will work that way.

I've cherry-picked about half of the changes from your branch (the POM-related ones). Unfortunately my test machine no longer can run Java 8. So I can't test out what I've done until I get a VM set up.

Jfyi, Java 11 is started here https://github.com/MachinePublishers/jBrowserDriver/tree/java11 but there are some showstoppers still.

Let me know your thoughts. Thanks for taking time with this to look into the issue and submit this!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MachinePublishers/jBrowserDriver/pull/354?email_source=notifications&email_token=AAOX7XLO6JXIBACYTK5KHY3QLQ7TZA5CNFSM4I2FALCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7UJ3FY#issuecomment-535338391, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOX7XJDRX7QWGIUEC4IA2TQLQ7TZANCNFSM4I2FALCA .

hollingsworthd commented 4 years ago

Yep when it's possible I'd rather make the code compatible with all versions of the dependencies. This is tested now and working with Selenium 3 and 4 and latest OpenJDK 8 that's packaged by Ubuntu 16.04. This is available now on Maven Central, version 1.1.1. Eventually with version 2 of jBrowserDriver it will move to support >= Java 11. And version 1.x will stay on Java 8 and Selenium 3/4.

The project is still specifying Selenium 4 as its dependency. You can downgrade in your own project by specifying these as your dependencies: https://search.maven.org/artifact/org.seleniumhq.selenium/selenium-api/3.141.59/jar , https://search.maven.org/artifact/org.seleniumhq.selenium/selenium-remote-driver/3.141.59/jar , https://search.maven.org/artifact/org.seleniumhq.selenium/selenium-server/3.141.59/jar ... And I like to exclude all the transitive dependencies of Selenium as this project doesn't need them. Not sure but it might also be ok to exclude selenium-server if you're not using it, which you probably aren't.

Thanks for bringing this up and the help!