Skip to content
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

[🚀 Feature]: deprecate WebElement.getAttribute #13334

Open
joerg1985 opened this issue Dec 19, 2023 · 17 comments
Open

[🚀 Feature]: deprecate WebElement.getAttribute #13334

joerg1985 opened this issue Dec 19, 2023 · 17 comments

Comments

@joerg1985
Copy link
Member

joerg1985 commented Dec 19, 2023

Feature and motivation

WebElement.getDomAttribute and WebElement.getDomProperty have been added in Selenium 4.

It might be good to deprecate WebElement.getAttribute to remove it at a undefined time in the future.

Usage example

This would warn people to move to the 'new' methods when maintaining existing code.

Copy link

@joerg1985, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

@titusfortner
Copy link
Member

Bold.

I'm not completely against the idea, but also not sure it's the right call.

@NCLnclNCL
Copy link

Feature and motivation

WebElement.getDomAttribute and WebElement.getDomProperty have been added in Selenium 4.

It might be good to deprecate WebElement.getAttribute to remove it at a undefined time in the future.

Usage example

This would warn people to move to the 'new' methods when maintaining existing code.

Let me ask, are the 3 APIs above different?

@titusfortner
Copy link
Member

getAttribute() assumes that people don't know the difference between a property and an attribute and don't want to be bothered to figure it out (a decent assumption), so it uses a bunch of fancy JavaScript that does a pretty good job of guessing whether the user is looking for the property or the attribute and returning that value. It's great convenience, but it is now implemented with a client side atom which means a large payload gets sent to the driver/server on every usage.

The ideal world people figure out if they want an attribute or property and use the correct method. But that might be asking a lot of users, and we already have the code right there...

@joerg1985
Copy link
Member Author

I was reading the blog post and the javadoc before opening the issue, this gave me the feeling the getAttribute method is the legacy way of reading attributes/properties.

If this is not true and there are no plans to ever remove the getAttribute method, this issue can be closed.

If this is a legacy method, it might be good to mark it deprecated or make a clear statement in the docs to ensure no new code is using this.
I agree there is alot of code using this, therefore a deprecation notice could be added, something like:
don't panic, this method will not be removed before selenium 6, but should not be used in new code.

@titusfortner
Copy link
Member

It would definitely be valuable to encourage people to use the other methods.

@titusfortner
Copy link
Member

We decided in TLC Meeting 1/18 that we would go ahead and mark this feature deprecated to encourage people to move to the more precise property and attribute methods.

We need a blog post explaining everything though, and I'm still not sure we want to actually remove it any time soon because it is quite useful.

Copy link

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the I-stale Applied to issues that become stale, and eventually closed. label Oct 26, 2024
@Delta456
Copy link
Contributor

We decided in TLC Meeting 1/18 that we would go ahead and mark this feature deprecated to encourage people to move to the more precise property and attribute methods.

We need a blog post explaining everything though, and I'm still not sure we want to actually remove it any time soon because it is quite useful.

Are we still doing this?

@github-actions github-actions bot removed the I-stale Applied to issues that become stale, and eventually closed. label Oct 27, 2024
@joerg1985
Copy link
Member Author

I hope so, i think this is still a valid point.

@Delta456
Copy link
Contributor

I can make a PR to deprecate WebElement.getAttribute and also help with the blog post if required.

@diemol
Copy link
Member

diemol commented Oct 28, 2024

It is still valid, just that no one has worked on it. Thank you for offering, @Delta456!

@Delta456
Copy link
Contributor

I will PR the deprecation but for the blog I will need your help @diemol and others.

@joerg1985
Copy link
Member Author

I just noticed something strage, which is probably the root for having getAttribute.
When an element is modified via javascript, e.g. call e.multiple=true on a <select>.
The getDomProperty changes, but getDomAttribute will stay null.
So client code should allways first check getDomProperty and then getDomAttribute.

This is a important point to the blog post in my mind.

@Delta456
Copy link
Contributor

Delta456 commented Nov 7, 2024

Across all language bindings the method has been deprecated. We now need to work on a blog post. I am up to volunteer for this.

VietND96 added a commit that referenced this issue Nov 26, 2024
diemol pushed a commit that referenced this issue Nov 26, 2024
…14808)

Revert "[py] Added Deprecation of WebElement.get_attribute() per #13334 (#14675)"

This reverts commit bb3053b.
@diemol
Copy link
Member

diemol commented Nov 29, 2024

If we are now going to ask users to do this #13334 (comment), then I think I am changing my mind, even if it was a TLC decision. We should be helping users and not asking them to do more. I am not sure what the real benefit of getting rid of this method is.

@canon-cmre-benoit-lecardonnel

Is there a blog post explaining how to migrate from getAttribute? We're now getting deprecation warnings, but I'm not sure what the difference between an attribute and a property is.
The Javadoc points to e.g. https://w3c.github.io/webdriver/#get-element-attribute, but that does not list usual property and attribute names.
Does #13334 (comment) mean that we should now call both and choose the first one that does not return null?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants