Skip to content

Conversation

@OmkarGharat
Copy link

@OmkarGharat OmkarGharat commented Mar 22, 2025

User description

Title:

Added getValue() method to WebElement

Motivation and Context:

Types of changes:

  • New feature (non-breaking change which adds functionality)

Checklist:

👉 Reviewers: Please let me know if any modifications are needed! 🚀


PR Type

Enhancement


Description

  • Added getValue() method to WebElement interface for retrieving element values.

  • Implemented getValue() in RemoteWebElement to fetch the value attribute.

  • Enhanced accessibility and programmatic access to element properties.


Changes walkthrough 📝

Relevant files
Enhancement
WebElement.java
Added `getValue()` method to `WebElement` interface           

java/src/org/openqa/selenium/WebElement.java

  • Introduced getValue() method to retrieve the value attribute.
  • Added detailed documentation for the new method.
  • Improved accessibility and usability of the WebElement interface.
  • +372/-291
    RemoteWebElement.java
    Implemented `getValue()` in `RemoteWebElement`                     

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

  • Implemented getValue() method to return the value attribute.
  • Utilized getDomProperty("value") for fetching the value.
  • +5/-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.
  • @CLAassistant
    Copy link

    CLAassistant commented Mar 22, 2025

    CLA assistant check
    All committers have signed the CLA.

    @qodo-merge-pro
    Copy link
    Contributor

    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

    Implementation Concern

    The getValue() method implementation uses getDomProperty("value") which might not be supported by all browsers or WebDriver implementations. Consider adding error handling or fallback to getAttribute("value") for better compatibility.

    public String getValue() {
      return getDomProperty("value");
    Documentation Mismatch

    The JavaDoc for getValue() mentions it's similar to getAttribute("value"), but the implementation in RemoteWebElement uses getDomProperty("value"). This inconsistency should be clarified in the documentation.

    * Gets the value attribute of the element.
    *
    * <p>
    * This method retrieves the current value of the element, similar to
    * {@code getAttribute("value")}. It is useful for getting text entered in
    * input fields or selected values in dropdowns.
    *
    * <p>
    * See <a href="https://www.w3.org/TR/webdriver/#get-element-attribute">W3C
    * WebDriver specification</a>
    * for more details.
    *
    * @return the value of the element as a string.
    */

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add nullable annotation

    The method declaration should specify that it may return null when the value
    attribute is not present, consistent with other attribute getter methods in the
    interface.

    java/src/org/openqa/selenium/WebElement.java [405-420]

     /**
      * Gets the value attribute of the element.
      *
      * <p>
      * This method retrieves the current value of the element, similar to
      * {@code getAttribute("value")}. It is useful for getting text entered in
      * input fields or selected values in dropdowns.
      *
      * <p>
      * See <a href="https://www.w3.org/TR/webdriver/#get-element-attribute">W3C
      * WebDriver specification</a>
      * for more details.
      *
    - * @return the value of the element as a string.
    + * @return the value of the element as a string, or null if the value is not set.
      */
    -String getValue();
    +@Nullable String getValue();
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies that the getValue() method should be consistent with other attribute getter methods in the interface by including the @nullable annotation. This is important for API consistency and proper null handling by consumers of the interface, as the value attribute might not be present for all elements.

    Medium
    • More

    @titusfortner
    Copy link
    Member

    Thank you for your contribution!

    The endpoint: /session/{session id}/element/{element id}/computedlabel

    is already implemented by getAccessibleName()
    https://github.com/SeleniumHQ/selenium/blob/123bb677/java/src/org/openqa/selenium/remote/RemoteWebElement.java#L158-L160

    While I can see the value of having a convenience method to get the value property in some use cases, it is easy to add in your own framework, and we're trying not to add simple wrapping methods to these already large classes.

    I'm going to close this, but thank you again for submitting a PR.

    @OmkarGharat
    Copy link
    Author

    OmkarGharat commented Mar 22, 2025 via email

    @titusfortner
    Copy link
    Member

    This is an example of how I wrap the WebElement from a workshop I teach:
    https://github.com/titusfortner/craft_framework/blob/test_fest/src/test/java/com/titusfortner/craft_framework/elements/Element.java

    I don't love all of this code, but it is useful for demonstration purposes. If I found myself doing a lot of getDomProperty("value") calls, I could see adding a convenience method to that class.,

    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