-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
RemoteWebDriver uses POST method for Get Active Element #2751
Comments
I think I have found the bug in the source code of AbstractHttpCommandCodec:
not? |
That looks like it, yes. But it needs to be special cased for when speaking to a W3C conformant remote as not to break backwards compatibility with Selenium remotes. |
My guess is that the other remotes just take anything, no matter if get or post. So I would suggest to just try it. If all of them are happy with “get”, then simply change it. |
I think that’s a bold assumption to make. I’m quite sure chromedriver is explicit about what HTTP methods it listens to, but I’m most willing to be proved wrong on this. |
Really? It sounds so weird to me to expect someone to “post” for getting some information. However, thanks for the feedback :) |
It is weird as you say, because this command doesn’t have any state-altering behaviour. This is incidentally why we’ve rectified it in the specification. I dug into this and it looks like chromedriver accepts only POST for |
It's post because it modifies server state. For w3c spec they decided to go
|
Now I’m curious. Which state is modified by “Get Active Element”? |
It's the same as find element: the server has to generate a web element
|
And that reference exists beyond the request? If so: why? Thanks for your patience with me :) |
That's how you interact with dom elements with webdriver. You get a web
|
How does it not alter state? See my previous comments. The server has to
|
It’s probably because I have no idea about how such a server works, but it sounds like an implementation detail to me if such reference / ID exists in the first place or is generated as a side effect of “Get Active Element”. |
To be clear, whether it alters state depends on the implementation. You could imagine an implementation that generates web element UUIDs for all DOM elements when it is under WebDriver control. Does it alter state in existing implementations? Well the most correct answer is “maybe”. If you ask for the active element and it has not been fetched previously, most drivers will hash it and create a new web element reference as jleyba explains. If it already exists in the set of known web elements it does not alter any state and it is a straight table lookup operation. However, I’m not sure I entirely agree that generating a web element reference amounts to “modifying server state”. The primitive that is accessed is |
Look at it another way: get active element is equivalent to I'm not trying to argue that the w3c should change from GET, just pointing out that the wire protocol's use of POST isn't as off base as has been suggested. |
The reason finding an element and executing scripts require POST is because we need to send a body, which is not the case for getting the active element, though. Of course I’m not arguing that any of this makes sense if you view this with RESTful goggles. I think it’s fair to say it’s an approximation to providing a “RESTish” API. 😄 |
fixed in java... needs fixing in the other languages as well |
This is a w3c spec conformance issue. Moved to the 4.0 milestone |
I’m confused. This is a huge blocker. How can anyone test with Firefox as long as this is not fixed? |
Ruby has been compliant from the beginning, and this is a duplicate: #2555 :) |
@andreastt Thank you! I’m happy to hear that postponing the issue doesn’t mean a rollback for the Java bindings. |
Fixed in .NET in 5e1d455 |
As is shown by the stacktrace in mozilla/geckodriver#223, Selenium uses the POST method when getting the current active element. It should use GET according to the specification.
I think this is a problem in the RemoteWebDriver, but I’m not sure.
The text was updated successfully, but these errors were encountered: