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

Should have an ActionChain click for Click Element keyword #1463

Closed
JonKoser opened this issue Sep 27, 2019 · 10 comments
Closed

Should have an ActionChain click for Click Element keyword #1463

JonKoser opened this issue Sep 27, 2019 · 10 comments
Labels
Milestone

Comments

@JonKoser
Copy link
Contributor

As it is now, the only way to use a native ActionChain-based click on an element is by using the Click Element At Coordinates keyword. It would be nice to not need to specify coordinates and just do a native click on the center of a given element. This has been implemented in PR#1462.

@aaltat
Copy link
Contributor

aaltat commented Sep 27, 2019

Hmmm, click at coordinates keyword has a special purpose, but I don't quite understand what problem you are trying to solve. Could you example clarify the why Click Element keyword is not suitable for your needs?

@JonKoser
Copy link
Contributor Author

My problem is that, for some reason due to asynchronous calls and promises, I'm unable to click on elements in my Chromium-based application using selenium's .click() function. It gives me this error:
unknown error: unhandled inspector error: {"code":-32000,"message":"Result of the evaluation is not a promise"} (Session info: content shell=)

Unfortunately, that's the method that Click Element uses. The way I've been able to get around it though is by calling a click action using ActionChains. The only keyword that uses the action chain method is Click Element At Coordinates.

@pekkaklarck
Copy link
Member

It would be good to investigate why that happens. Perhaps there's a bug somewhere that could be fixed. If not, there still are other solutions than adding a new keyword:

  • Change Click Element and possibly also other Click keywords to use action chains.
  • Change them to use action chains if click() fails.
  • Add an option like action_chain=False to make this behavior configurable.

@JonKoser
Copy link
Contributor Author

JonKoser commented Sep 28, 2019

That's fair. In lieu of finding the cause of the failure, since that might take some time, is there one of those options you think might be a good stop-gap? I feel like using action chains if click() fails or adding the option are both good routes. I see no reason to change everything to use action chains. I'll close my PR until this is decided.

@JonKoser
Copy link
Contributor Author

I have a branch where I created a helper function which centralizes click actions (like in Click Button, Click Link, Click Image, Click Element). It first tries to click the web element with web_element.click(). If that attempt fails with a WebDriverException it then tries to click the element with an ActionChain click. Seems to work pretty darn well. If you like that approach I'd be happy to create a PR based off that. Otherwise, I'd be happy to try something else.

@aaltat
Copy link
Contributor

aaltat commented Sep 28, 2019

Hmmm, that is actually quite a difficult question. Usually we don't do retry (expect in the Wait... keywords, but that is those keyword purpose.) If we do automatic retry it might actually hide some real bugs from the user. Like, first click doesn't work for some reason, but second one works and because we automatically retry that is hidden. But it might be also useful feature to retry.

I am somewhat leaning towards that configurable option would be better for users. Because it's explicit what keywords will do and easier users to control. Someone most likely wants to keywords to try clicking only one time and fail.

@pekkaklarck
Copy link
Member

Good point that clicking twice may not always be desirable and thus configuration option sounds better. Even better, of course, if the reason for this behavior was found.

If a configuration option is added, I'd say it's best to add it only to Click Element and not to other click related keywords. It feels a bit like a hack anyway, and perhaps future changes to Selenium, browser driver or browser make it unnecessary.

@aaltat
Copy link
Contributor

aaltat commented Sep 28, 2019

So we agree that keyword signature will change to:
| Click Element | locator | modifier=False | action_chain=False |. Retry is not done and keyword will fail at the first attempt.

@pekkaklarck
Copy link
Member

That can be considered ugly and hackish, but perhaps practicality beats purity.

@JonKoser
Copy link
Contributor Author

Agreed! A bit hackish, maybe, but it'll also be nice just to have the option since the to types of clicks clearly exercise the code in different ways.

@aaltat aaltat changed the title Should have a native (ActionChain) click for elements without an offset Should have an ActionChain click for element Oct 7, 2019
@aaltat aaltat closed this as completed in 74204ec Oct 7, 2019
@aaltat aaltat added the acknowledge To be acknowledged in release notes label Oct 7, 2019
@aaltat aaltat added this to the V4.1.0 milestone Oct 7, 2019
@aaltat aaltat changed the title Should have an ActionChain click for element Should have an ActionChain click for Click Element keyword Oct 8, 2019
@aaltat aaltat added the rc 1 label Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants