Skip to content

[feature] add method RemoteWebDriver.isDownloadsEnabled()#15868

Merged
diemol merged 2 commits intoSeleniumHQ:trunkfrom
asolntsev:feature/add-method/is-downloads-enabled
Jun 6, 2025
Merged

[feature] add method RemoteWebDriver.isDownloadsEnabled()#15868
diemol merged 2 commits intoSeleniumHQ:trunkfrom
asolntsev:feature/add-method/is-downloads-enabled

Conversation

@asolntsev
Copy link
Copy Markdown
Contributor

@asolntsev asolntsev commented Jun 5, 2025 •
edited by qodo-code-review bot
Loading

User description

💥 What does this PR do?

I want to add method RemoteWebDriver.isDownloadsEnabled() which re

🔧 Motivation

I need to check if current webdriver has capability se:downloadsEnabled.

My scenario looks like this:

  public File downloadFile(HasDownloads webdriver) {
    if (webdriver.isDownloadsEnabled()) {
      return webdriver.getDownloadableFiles().get(0);
    }
    else {
        // click element, look into "Downloads" folder etc.
    }
  }

🔄 Types of changes

  • New feature
  • Breaking change
    • any 3-rd party webdrivers that implement HasDownloads, but don't extend RemoteWebDriver, will need to implement method isDownloadsEnabled().

PR Type

Enhancement

Description

  • Add isDownloadsEnabled() method to RemoteWebDriver

  • Require HasDownloads implementors to define isDownloadsEnabled()

  • Refactor download capability check in HasDownloads

Changes walkthrough 📝

Relevant files
Enhancement
HasDownloads.java
Add and refactor downloads-enabled capability checks in HasDownloads

java/src/org/openqa/selenium/HasDownloads.java

  • Add abstract isDownloadsEnabled() method to interface
  • Add static utility for capability check
  • Refactor requireDownloadsEnabled to use new method
  • Update documentation for new method
    		• +11/-2   
    RemoteWebDriver.java
    Implement isDownloadsEnabled() in RemoteWebDriver               

    java/src/org/openqa/selenium/remote/RemoteWebDriver.java

    • Implement isDownloadsEnabled() using HasDownloads utility
    		 +4/-0     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @asolntsev asolntsev requested a review from diemol June 5, 2025 16:59
    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review bot commented Jun 5, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Interface Compatibility

    The PR adds a new abstract method to the HasDownloads interface without a default implementation, which will break existing implementations that don't extend RemoteWebDriver. Consider providing a default implementation that uses the static utility method.

     
    boolean isDownloadsEnabled();

    @qodo-code-review
    Copy link
    Copy Markdown
    Contributor

    qodo-code-review bot commented Jun 5, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add missing annotation

    Add the @OverRide annotation to the isDownloadsEnabled() method since it
    implements a method from the HasDownloads interface. This improves code clarity
    and allows the compiler to verify correct implementation.

    java/src/org/openqa/selenium/remote/RemoteWebDriver.java [652-654]

    +@Override
 public boolean isDownloadsEnabled() {
   return HasDownloads.isDownloadsEnabled(capabilities);
 }
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion correctly identifies that the @Override annotation should be added since isDownloadsEnabled() implements an interface method from HasDownloads. This is a minor but valid code style improvement.

    		Low
    • Update

    @asolntsev asolntsev force-pushed the feature/add-method/is-downloads-enabled branch from 50ee651 to 26bf2d0 Compare June 5, 2025 18:00
    @asolntsev
    Copy link
    Copy Markdown
    Contributor Author

    asolntsev commented Jun 5, 2025

    Add the @OverRide annotation to the isDownloadsEnabled()

    Thanks, fixed.

    @nvborisenko
    Copy link
    Copy Markdown
    Member

    nvborisenko commented Jun 5, 2025

    That's something strange. Today isDownloadsEnabled(), tomorrow isUploadEnabled(). Who enables it? I want to believe it should be determined by the initiator. And core question who is "initiator".

    @asolntsev
    Copy link
    Copy Markdown
    Contributor Author

    asolntsev commented Jun 6, 2025

    That's something strange. Today isDownloadsEnabled(), tomorrow isUploadEnabled(). Who enables it? I want to believe it should be determined by the initiator. And core question who is "initiator".

    I absolutely agree with you: in a typical "test automation" project, the initiator knows.
    But my case is different: I want to check it in a generic testing library.

    I am implementing "file download" method. I receive the webdriver from end-user, and I need to check if the downloads are enabled.

    @diemol diemol merged commit eff4a30 into SeleniumHQ:trunk Jun 6, 2025
    4 checks passed
    @asolntsev asolntsev deleted the feature/add-method/is-downloads-enabled branch June 6, 2025 13:46
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @diemol diemol diemol approved these changes

    Assignees

    No one assigned

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @asolntsev @nvborisenko @diemol