raise an error in case if user tries to search using XPATH

[beliaev-maksim]

Description

Fail execution fast in case if user tries to get shadow element by unsupported XPATH

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[titusfortner]

The problem is that XPath is supposed to be supported. The same as TAG_NAME. I know we've done it in other places, but I don't like the practice of throwing errors in Selenium for things that the drivers just haven't implemented yet.

[beliaev-maksim]

@titusfortner what if we replace to not implemented error, stating that it will come in future versions?

Because now it is super confusing))

[titusfortner]

What is the current error?

[beliaev-maksim]

@titusfortner
I mean per current PR I proposed value error (since we do not accept XPATH)

while according to your comment this is temporary and in the future we should provide this functionality. Thus, we can in meantime raise NotImplementedError and tell users that they can expect this functionality in the future, but currently they have to use other means to get the element

[titusfortner]

I don't think Selenium should be coding around bugs in the drivers. The error from the driver should be good enough to explain what is happening, and if it isn't, the driver should give a better error. Which is why I asked what is the current error that you want to make better.

[beliaev-maksim]

@titusfortner oh, I see what you mean

it is "selenium.common.exceptions.InvalidArgumentException: Message: invalid argument: invalid locator"

I agree that in this case locator is referred to XPATH. But the message is not trivial and user friendly. If you are used to work without shadow elements and it is your first implementation with selenium, then, we can provide more guidance on what to use rather then pushing users to read the source code

[titusfortner]

Ok, I filed this with geckodriver - mozilla/geckodriver#2105

I'll leave this open in case someone else disagrees with me (@diemol ?) and thinks it's fine to block specific functionality based on driver support.

[beliaev-maksim]

@titusfortner
Well, I received this error with Chrome

Geko doesn't support shadow dom in any form

So, I still think it is on selenium side :)

[titusfortner]

It does in Nightly! (or maybe Beta now?) Anyway, not sure why I thought it was Firefox.

[beliaev-maksim]

So, if all the drivers don't support the method, then we can block the usage on selenium side

Or what is the best solution?
Because now it is really cryptic and we for sure can make more for users (in terms of messaging)

[beliaev-maksim]

@titusfortner also, if I am not mistaken user will get an exception from Selenium if tries to use shadow dom on any browser except Chrome

so, we already filter and report functionality on selenium side

[diemol]

Could you point where we are filtering the browser? I guess we need to apply the fix across all bindings.

[beliaev-maksim]

@diemol sure.

selenium/py/selenium/webdriver/remote/webelement.py

Line 246 in 86a5b0d

assert (

I see that support for Firefox was added in the latest. But it does not change the overall idea from comment: #11893 (comment)

[diemol]

We need to fix this across bindings. We are not going to merge this until we decide how to proceed across languages. However this PR is great since it gets the conversation started. We will update what we decide soon.

[titusfortner]

user will get an exception from Selenium if tries to use shadow dom on any browser except Chrome

This is not true in all of the languages. Each language is doing slightly different things with how it is supporting different features (Ruby mixins, Java interface implementations). Some of this design is a hold-over from when there wasn't a specification and we were ok with it being the wild-west on what things were supported.

If it's in the w3c, I think the associated method should be available to all driver classes, and for the drivers themselves to throw a meaningful response. I don't think Selenium should be tracking when drivers implement which things; and if there is a problem, it should be obvious that it's a driver issue not a Selenium issue.

[diemol]

I created #12122 to remove that browser filtering because Python was the only one doing it.

During the meeting on 25.05.2023 it was decided add support for everything in spec and let the drivers error when things do not work.

[beliaev-maksim]

@diemol that makes perfect sense!

How can we engage webdriver community to be more specific on errors they provide?
Ideally, that should be standartized across different drivers

[titusfortner]

Well, ideally there are no errors because everything is supported. 😄

The reason something doesn't work can vary quite a bit between browsers, though, so there isn't much to standardize. If there's an error from a driver that doesn't make sense, we file a bug on the respective issue tracker.

[beliaev-maksim]

@titusfortner gotcha!
Thanks!