Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Selenium driver shouldn't return every executed script #71

Closed
jamuraa opened this Issue · 10 comments

4 participants

@jamuraa

Selenium can't return complex Javascript objects, so when you try to execute short scripts that return objects (jQuery objects are an excellent example), the driver breaks with "too much recursion".

Can be fixed by replacing "return #{script}" with "#{script}" in Capybara::Driver::Selenium.execute_script

@joshuacronemeyer

Thanks for highlighting this issue! Had this same problem, but it is hard to track down because it was causing firefox to hang so you have no idea what kind of error is actually happening... trick was opening the error console manually before the evaluate_script gets called.

Thanks jamuraa. i voted up this issue and hope somebody merges your patch soon.

@joshuacronemeyer

fyi, it has come up on the mailing list that this is an upstream defect with selenium when it tries to convert complex javascript objects to ruby. if they are fixing this upstream then that will address this.

@jnicklas
Owner

Problem is that we are losing valuable functionality by not returning from the call. I'd rather see this fixed upstream. Closing this since it's not a Capybara bug, if someone wants to tackle this, please open a ticket on the Selenium tracker.

@jamuraa

This is fine, but it may be a long time before Selenium has a fix for this issue. It took me about 2 hours to figure out the cause because of the bad error, and the hidden return. I did open a bug on the selenium tracker for the error message being quite confusing.

You can still return the value (if it is returnable) by returning from the script itself. I guess the issue is more that the javascript being executed is not what I was expecting - I expected the script I passed to capybara to be executed, and instead "return #{script}" was executed instead.

I haven't tested this, but does this break code that you would otherwise expect to run, like execute_script("item = add_item(); set_default_properties(item)")? Will item get returned? will set_default_properties() get called?

Also, the culerity driver doesn't return the value, so there is at least a consistency issue.

@jnicklas
Owner

Afaik the culerity driver does return the value. At least there's a test case for it that's currently passing.

@jarib

What valuable functionality is lost by not adding the return?

Also, what would you expect back from Selenium in this case? What would be a sane ruby representation of a JQuery object? I'm afraid returning anything that can't be passed through JSON.stringify() will be quite difficult in a real browser.

In Celerity, the situation is a little different since all JS objects are already in memory as Java/Rhino objects, so HtmlUnit simply returns a reference to that.

@jnicklas
Owner

What about the following solution, we have evaluate_script, how about adding a second method called execute_script, which does not add the return. That way people can choose which one they want to use and use evaluate_script when they do need the return value. I think it makes sense from a naming point of view?

@jamuraa

That would be fine for making things work, but I am wondering why you are against the idea that the user could just include a "return" of their own in the script that they pass to evaluate_script in order to retrieve a value. Does the culerity driver make it impossible to not return the value?

My major concern was that there was hidden javascript being executed which I didn't know, and that caused a loss of about an hour of digging trying to find a bug in my code that wasn't even the problem before I even thought it might be something with Selenium and capybara.

@jnicklas
Owner

My concern is that it would be more difficult to get consistent behaviour across drivers. Some drivers may always return, while others won't. The solution with adding a return to the script works for Selenium, afaik it doesn't do anything on Celerity and we have no idea how it would behave on hypothetical other drivers.

Imho the return statement does not make sense from a JavaScript point of view. You are not executing a function. That is the only place where return is used in JavaScript. To have it used in this way does not make semantic sense to me, and is at best unexpected. Can you honestly say that your first instinct upon discovering that no value was returned would have been to add the return statement to the JavaScript? Mine definitely wouldn't have.

All of you are not surprised that the script returns a value, you are all surprised by the bug encountered in Selenium. If that didn't exist you wouldn't have thought twice about this.

@jamuraa

My first instinct was that there would be no connection at all from the JavaScript back, and I was very surprised to find that the JavaScript that I was passing to execute (evaluate, whatever they are synonyms to an extent) wasn't being passed verbatim.

At this point the google results for "capybara too much recursion" pointing here as well as Issue #76 is probably enough until Selenium figures something out.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.