[java][BiDi] implement web extensions
User description
🔗 Related Issues
Implements https://github.com/SeleniumHQ/selenium/issues/15585
💥 What does this PR do?
Implements https://github.com/SeleniumHQ/selenium/issues/15585 for Java binding
🔧 Implementation Notes
The PR will fail on stable firefox and needs firefox to be 138 version as stated in https://github.com/SeleniumHQ/selenium/issues/15585#issuecomment-2782657812
💡 Additional Considerations
🔄 Types of changes
- New feature (non-breaking change which adds functionality and tests!)
PR Type
Enhancement, Tests
Description
-
Add BiDi web extension install/uninstall API for Java
- Implements
webExtension.installandwebExtension.uninstallcommands - Supports extension install via path, archive, or base64
- Implements
-
Introduce new classes for extension data handling
-
Add comprehensive tests for extension installation methods
-
Update Bazel build files for new module and test integration
Changes walkthrough 📝
| Relevant files | |||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement | 7 files
| ||||||||||||||
| Configuration changes | 5 files
| ||||||||||||||
| Tests | 1 files
|
Need help?
Type /help how to ...in the comments thread for any questions about Qodo Merge usage.Check out the documentation for more information.
PR Reviewer Guide 🔍
Here are some key observations to aid the review process:
|
🎫 Ticket compliance analysis ✅ 15585 - PR Code Verified Compliant requirements:
Requires further human verification:
|
| ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ |
| 🧪 PR contains tests |
| 🔒 No security concerns identified |
⚡ Recommended focus areas for reviewVariable Naming Inconsistency
|
PR Code Suggestions ✨
Explore these optional code suggestions:
| Category | Suggestion | Impact |
| Learned best practice |
Add parameter null checkAdd a null check for the path parameter in the constructor to prevent java/src/org/openqa/selenium/bidi/webextension/ExtensionArchivePath.java [25-27]
Suggestion importance[1-10]: 6__ Why: Relevant best practice - Add null checks for parameters and properties before using them to prevent NullReferenceExceptions. | Low |
| General |
Fix variable naming conventionVariable naming convention is inconsistent. The variable java/test/org/openqa/selenium/bidi/webextension/WebExtensionTest.java [53-57]
Suggestion importance[1-10]: 5__ Why: The suggestion correctly identifies and fixes a violation of Java variable naming conventions, improving code readability and maintainability, but it does not affect functionality or correctness. | Low |
| ||
@Delta456 webextensions for Python are landing soon. It would be great to merge this also. Are you still just waiting for review? Can you drop a note on slack to ask for a reviewer?
@Delta456 webextensions for Python are landing soon. It would be great to merge this also. Are you still just waiting for review? Can you drop a note on slack to ask for a reviewer?
Yes. I am still waiting for review. I have already asked on slack but I will do it just in case.
I would help, but my Java is about as good as my Swahili
It's been a bit since I coded much in Java either lol, but after a cursory look it LGTM
Could you please add some JavaDoc or explain in the PR description the approach you are using?
I had to read the code a few times to understand why we have
InstallExtensionParametersandUninstallExtensionParameters. Somehow it seems like many classes to do this. Perhaps after reading the approach you are using I would understand this better.
I have used a class-based approach for InstallExtensionParameters and UninstallExtensionParameters because this has been used for other BiDi modules. I know there are so many for that, but I don't have a solution on how it can be reduced more.