Skip to content

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

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

Merged

Conversation

asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Jun 5, 2025

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.
  • Copy link
    Contributor

    qodo-merge-pro 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();

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 5, 2025

    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
    Contributor Author

    Add the @OverRide annotation to the isDownloadsEnabled()

    Thanks, fixed.

    @nvborisenko
    Copy link
    Member

    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
    Contributor Author

    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
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants