-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
added get dom property and get dom attribute method information #2529
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
base: trunk
Are you sure you want to change the base?
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
✅ Deploy Preview for selenium-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The tests failure doesn't look related to this PR. Kindly check this. |
| ## Fetching Attributes or Properties | ||
| ## Fetching Attributes and Properties | ||
|
|
||
| ### Get Attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we need to be more precise now with the difeerences between getAtrribute, getDomAttribute and getDomProperty
example:
Returns the element's property value if present, otherwise falls back to the attribute value. This convenience method handles the common case but can be ambiguous. For precise control, use getDomProperty() or getDomAttribute() instead.
@diemol any thoughts !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are showing in here how to use it. Let me know if any changes are needed at code level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harsha509 getAttribute() is actually deprecated for just that reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including a synopsis of this information would be helpful: What's the difference between an "attribute" and a "property" in the DOM? | Quiz Interview Questions with Solutions https://share.google/lB3bUx9KDK28LOx9B
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sbabcoc,
I’m already aware of the distinction, this is exactly why I asked for the documentation to clearly outline the difference in a way users can understand.
Also getAtrribute is not deprecated yet in selenium see https://github.com/SeleniumHQ/selenium/blob/trunk/java/CHANGELOG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am unable to understand the discussion and reasoning around the new information added about new methods at document and code level, and it seems to me i am repeating myself again. if this PR isn't correct then don't add it. i don't know what to do in here further. im finding this exhausting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the regular process of contributing to a project to send a PR. Sometimes people don't understand the context. They ask questions, change the context, and then follow up by asking more questions. Some PRs stay open and in discussions for weeks, sometimes months. It's not ideal, but that's just the way it is.
Sometimes, it's very quick. Sometimes it takes a bit longer. I'm very happy that you're taking the time to contribute. I hope you understand that sometimes it doesn't go through at the first try. You get feedback, and then we iterate on it. It's not perfect, but it's a collaborative process. This fosters a sense of working as a team, and without feedback, there would be no point in open source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rpallavisharma ,
There is nothing wrong in this PR and I totally understand the intention behind the PR and appreciate the additions.
My point is that introducing getDomAttribute() and getDomProperty() without first clarifying the ambiguity of getAttribute() may confuse users even more.
The docs should reflect the real behavior of getAtrribute, since ambiguity affects users in practice.
So, as a final suggestion from my review, please update the getAttribute() explanation to something like:
getAttribute Returns the element's property value if present, otherwise falls back to the attribute value. This convenience method handles the common case but can be ambiguous. For precise control, use getDomProperty() or getDomAttribute() instead.
Once that is clearly stated, the existing code example can be adjusted accordingly, and we won’t need separate sections for getDomAttribute() and getDomProperty().
Also, here is the additional info, regarding the ambiguity for your reference:
- getAttribute() is not a W3C command — it executes JavaScript internally (js atoms), so its behavior differs.
- getDomAttribute() and getDomProperty() are W3C-compliant commands.
Hope this clarifies everything!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harsha509 you don't want to have section for the new commands added separately in document?
Only get attribute which already exists should be there? And modified to what you wrote?
Then the code for the new commands where does that go?
In that case this PR doesn't do it.
It added information for new commands with code examples.
@diemol sure we can continue this discussion , as much as necessary. Fine with me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are three methods get attribute get dom property get dom attribute . these are three different methods. we kept get attribute for backward compatibility, and added two new.
i created that PR to showcase two new and code examples for it. didn't change get attribute.
this blog - https://saucelabs.com/resources/blog/selenium-4-new-element-attribute-and-property-methods
the problem now is that you are saying we update get attribute definition as well? or we add the information of these two new methods in same method?
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
User description
Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, and help reviewers by making them as simple and short as possible.
Added information of Get Dom Property and Get Dom Attribute.
Provided code for java, added for all languages
Description
Added information of Get Dom Property and Get Dom Attribute.
Provided code for java, added for all languages
Motivation and Context
information about these methods were missing
Types of changes
Checklist
PR Type
Documentation, Enhancement
Description
Added
getDomProperty()andgetDomAttribute()method documentationImplemented Java test examples for both new methods
Updated documentation across multiple language versions (English, Japanese, Portuguese, Chinese)
Reorganized section header from "Fetching Attributes or Properties" to "Fetching Attributes and Properties"
Diagram Walkthrough
File Walkthrough
InformationTest.java
Add getDomProperty and getDomAttribute test examplesexamples/java/src/test/java/dev/selenium/elements/InformationTest.java
getDomProperty("value")methodgetDomAttribute("value")methodinformation.en.md
Add DOM property and attribute method documentationwebsite_and_docs/content/documentation/webdriver/elements/information.en.md
documentation
reference
reference
Kotlin
information.ja.md
Add DOM property and attribute documentation in Japanesewebsite_and_docs/content/documentation/webdriver/elements/information.ja.md
documentation
reference
reference
languages
information.pt-br.md
Add DOM property and attribute documentation in Portuguesewebsite_and_docs/content/documentation/webdriver/elements/information.pt-br.md
documentation
reference
reference
information.zh-cn.md
Add DOM property and attribute documentation in Chinesewebsite_and_docs/content/documentation/webdriver/elements/information.zh-cn.md
reference
reference
Kotlin