Allow arguments to be passed into JavaScript scripts

Selenium allows arguments to be passed along with JavaScript that can be referenced in the script using the injected List variable named arguments. Execute Javascript, Execute Async Javascript, and Wait For Condition do not support this feature. This would be useful for passing WebElements and data structures into scripts

When we implement Get Webelement(s), people will naturally want to pass the WebElements as argument(s) to a script. Currently, one can use Assign Id To Element to make a WebElement easy to reference if JS if it is difficult to reference is JS.

Implementing this without breaking test cases or adding keywords would require using keyword arguments for Execute Javascript and Execute Async Javascript. Because they have variable arguments, we cannot add a positional argument. Keyword arguments require RF 2.8 to work.

There is a workaround for this missing functionality on StackOverflow

Requiring RF 2.8 shouldn't be a problem because it's not that new release anymore. I don't think minimum RF requirement needs to be raised, though, but obviously that needs to be documented in kw doc.

@pekkaklarck

After getting far into adding this feature using keyword arguments, I realized there is a serious backwards incompatibility issue. Assignments or comparisons in JavaScript are triggering Robot Framework's free keyword argument processing. Example:
Execute Javascript var elem = document.getElementById('foo'); elem.selected = true;
Fails with message 'var elem ' is not a valid keyword argument. The equals sign would have to be escaped:
Execute Javascript var elem \= document.getElementById('foo'); elem.selected \= true;
I pushed a commit if you want to see the progress thus far. I am not going to do anything more until we have a consensus.

I see three options to implement this feature:

Implement with keyword arguments, breaking many scripts and always needing to escape '='s
Remove the capability to split the code in multiple cells (change sig to execute_javascript(code, *args)). This will break tests that split the code (far less than option 1), unfortunately with the vague error WebDriverException: Message: u'JavaScript error'.
Add two keywords Execute Javascript With Args and Execute Async Javascript With Args that have the arguments (code, *args), thus not supporting breaking the script into multiple cells. This would not break any test but certainly adds to keyword clutter.

@pekkaklarck @emanlove @j1z0 I want hear your opinions and anyone else's on this. Maybe we should post to the user group and have a poll like Ed mentioned?

I just thought of one thing that would make option 2 less painful. If arguments were passed and code does not contain the string arguments then fail with message Arguments were passed and not referenced. Splitting the script into multiple cells is no longer supported

@ombre42

Comments to analysis by @ombre42:

The motivation to allow splitting the script to multiple arguments in old SeleniumLibrary was to ease executing longer scripts. This was especially important in old HTML test data days when you couldn't easily have a long string even in one row due to columns. Even today splitting multiple lines of JavaScript like below can be handy:

*** Test Cases ***
Example
    Execute Javascript
    ...    var foo = 'example';
    ...    var result = doSomething(foo);
    ...    if (result == 42)
    ...        // and so on

Passing arguments to JavaScript sounds like a nice feature. Because longer JavaScript code can be easily constructed using Catenate keyword, losing the above ability to pass the code in pieces shouldn't be that big a problem in general.
It would be good to avoid backwards incompatible changes is possible. @ombre42 already presented an idea how to detect a situation where user has passed the code in pieces. Instead of failing, it would be better to emit a deprecation warning and behave like earlier, though. Alternatively we could deprecate passing the code in multiple pieces first and add this functionality only in the next release.
The requirement to escape = makes using kwargs pretty much a no-go in this particular use case. My trivial example above has four equal signs already.
Would a good signature be:

def execute_javascript(code, arguments):
    # ...

or

def execute_javascript(code, *arguments):
    # ...

The latter would have the same problem as the current signature if the keyword ever needs more arguments. It would be a little more convenient than creating a list first, though, but if it's fine to require the code to be given as one and not in pieces, that shouldn't matter too much either.

From backwards compatibility point of view, the former signature would cause problem if the code is passed in as more than two pieces. It would thus need to be something like:

def execute_javascript(code, arguments, *deprecated):
    # ...

Thank you so much for your input Pekka. I never understood why the code-split feature. We use RIDE exclusively where I work and any formatting like you mentioned will be re-written by RIDE.
Maybe I'm short-sighted, but I do not foresee adding arguments to this in the future, so I'm leaning towards this solution, which can go into 1.6:
Change to def execute_javascript(code, *arguments):

If len(arguments) > 0
   If code contains the string 'arguments'
       process arguments as arguments to the script
   else
       process arguments as extra code to catenate with code and script will have no arguments
       log a warning
else
    run script with have no arguments

I think the probability of a script containing the string 'arguments' but not intending to reference the injected arguments from Selenium is quite low, but the logic must be described in the keyword doc.

We could also change it to
def execute_javascript(code, *arguments_or_code)
And continue support the ability to split code indefinitely based on wheter code contains the string arguments (and not log warnings if it doesn't). This is a little awkward, but worth mentioning.

Hmm, execute_javascript(code, *arguments_or_code) is definitely awkward but also a pretty clever idea. Not losing the ability to split long code and thus also avoiding backwards-incompatible changes (except if code contains arguments) would be a big win. I don't see why this wouldn't work if it is clearly documented with examples.

@emanlove

@emanlove - Thoughts? The reason behind *arguments_or_code was that there does not seem to be much gained by removing the backwards compatibility code. That, and warnings should be used with caution. Warning about not supporting functionality in a future version is not good unless you have a plan on when and why.

@ombre42

@ombre42: I'm still reading through this and thinking about different solutions. I tend to think having arguments will be useful and a functionality that we would want to add. There will be an issue with using your idea in that the string 'arguments' is what the javascript uses within the script to 'call' the argument. For example see some of my AngularJS additions. It is a great idea by the way but we would need a different separator string.

Looking at the examples above I noticed the Javascript code is not delineated. For some reason, possibly because my recent use of Python webdriver, I was expecting quotes around the script to be executed. Why not use this as the delimator or separator between script and arguments. If the first argument begins and ends with a quotation mark then this marks the script and the remaining arguments are arguments for the javascript. So for example the second execute javascript call in the following RF test does not create an alert box; it is simply an unassigned javascript string.

Execute Javascript With Quotes
    [Documentation]  Testing out exec Javascript with Arguments
    Execute Javascript  alert('Hello World!')
    Alert Should Be Present  text=Hello World!
    Execute Javascript  "alert('Hello World!')"
    Run Keyword And Expect Error  *  Alert Should Be Present  text=Hello World!

Here one could do something like ...

Execute Javascript With Arguments
    [Documentation]  Testing out exec Javascript with Arguments
    Execute Javascript  alert('Hello World!')
    Alert Should Be Present  text=Hello World!
    Execute Javascript  "alert('arguments[0]');"  ${argument1}  ${argument2}
    Alert Should Be Present  text=${argument1}

This might be an alternative to your use of the 'arguments' string, Kevin. We could even get fancy and count quotes doing something like

Execute MultiLine Javascript With Quotes
    [Documentation]  Testing out exec Javascript with Arguments
    Execute Javascript  alert('Hello World!')
    Alert Should Be Present  text=Hello World!
    Execute Javascript  "alert('Hello World!');
    ...    var foo = 'example';
    ...    var result = doSomething(foo);
    ...    if (result == arguments[1])
    ...        // and so on    ..."
    ...    argument1
    ...    argument2

In trying to see if an unassigned string literal would conflict in anyway with the Javascript specification I did come across the "use strict"; directive and more generic concept of Directive Prologues. Noting in the specifications for Directive Prologues that the semi-colon is not required explicitly it would be possible then for someone to write a test case like

Execute MultiLine Javascript With Use Strict
    [Documentation]  Testing out exec Javascript with Arguments
    Execute Javascript  "use strict"
    ...    var foo = 'example';
    ...    var result = doSomething(foo);
    ...    if (result == arguments[1])
    ...        // and so on    ...

and not intend to use the Execute Javascript keyword as I propose. But I think this a minor case that can be handle by 1) checking for common directive prologues (although I can't seem to find a list of common ones nor do I think they are explicitly defined; i.e. anyone create create custom ones) and 2) users can add a semi-colon to the end thus avoiding our assumption that this is a code block and the remaining arguments are javascript arguments. We could also provide some sort of warning.

After thinking about possible solutions a little more, simply implementing new Execute Javascript With Arguments and Execute Async Javascript With Arguments start to sound pretty tempting.

Pros:

No backwards-compatibility issues whatsoever.
Clean and easy to document API.

Cons:

Two more keywords.
New keyword names are pretty long.

@emanlove

@emanlove Thanks for pointing out that the "arguments" trick won't work. I was not thinking of async JS where it would always be in the script to call the callback.
If we are going to add keywords, I think "args" is an acceptable abbreviation to make the keyword name a little shorter - its OK for PyCharm's spell-check and its used in the DatabaseLibrary.
I still think deprecating splitting the script for one version and changing to (code, *arguments) or (code, *args) is still on the table.
If we are going to add keyword(s), we could do exucute_javascript_with_args(code, async='False', *args) but that essentially makes async a mandatory argument and I do not like that.
I am glad both of you are so concerned about finding the ~~right~~ best solution as everything we have come up with so far has been a comprimise.

@ombre42

I'd like to revive this conversation as it seems like some people may need it.

I like some of the ideas @ombre42 had. I think I have an idea that would kinda fit in the best of both worlds but it's still a little hacky.

I'm imaging something like this

def execute_javascript(args, code*):
  if len(code) == 0:
    code.append(args)
  elif isstr(args):
    code.insert(0, args)
  elif isinstance(args, list):
    browser.execute_script(code, args*)
  else:
    browser.execute_script(code, args)

(Look over my really rough psudo-code. I think you get the idea.)

So, it is a little hacky. If the argument is a string then it must be added to a list before it can be passed through. Regular objects can be passed in like normal though (like web elements). Multiple arguments have to be passed through an array. If there's no args then it's called exactly how it is today.

What do you think @pekkaklarck, @emanlove?

Sure, I can do that. Thanks for your appreciation!

By a quick look, I thought we had a design in the issue, but after some more reading we don't actually have a design. At least I don't know what should I expect in the OR. Before implementing the PR, we should have a rough idea about the design. And before that I should read how JavaScript arguments actually work.

We did agree the design in the #1147 PR. In short it is this:

Support current style:

Execute JavaScript
...    element = arguments[0];
...    original_style = element.getAttribute('style');
...    element.setAttribute('style', original_style + "; background: yellow;");
...    setTimeout(function(){
...       element.setAttribute('style', original_style);
...    }, 300);

Support arguments in the end:

Execute Javascript
...    element = arguments[0];
...    original_style = element.getAttribute('style');
...    element.setAttribute('style', original_style + "; background: yellow;");
...    setTimeout(function(){
...       element.setAttribute('style', original_style);
...    }, 300);
...    ARGUMENTS
...    ${element}    ${unused_arg}

Support changing the order of arguments and code:

Execute Javascript
...    ARGUMENTS
...    ${element}    ${unused_arg}
...    JAVASCRIPT
...    element = arguments[0];
...    original_style = element.getAttribute('style');
...    element.setAttribute('style', original_style + "; background: yellow;");
...    setTimeout(function(){
...       element.setAttribute('style', original_style);
...    }, 300);

Support defining the JAVASCRIPT at start (optional)

Execute Javascript
...    JAVASCRIPT
...    element = arguments[0];
...    original_style = element.getAttribute('style');
...    element.setAttribute('style', original_style + "; background: yellow;");
...    setTimeout(function(){
...       element.setAttribute('style', original_style);
...    }, 300);
...    ARGUMENTS
...    ${element}    ${unused_arg}

For more details see the discussion in #1147

Looks good. What's the use case for JAVASCRIPT marker?

Option number 3, when markers are swapped.

Well, what's use case for that? =)

I had a discussion and sometimes it's feels more natural to define the arguments first in JavaScript. For me it feels, when looking from Python side, totally unnatural, but then again I am not that familiar with JavaScript.

I found that odd order as well. If someone finds it useful, and is willing to implement it, I don't see much problems with it, though. Only somewhat serious drawback is making the documentation of the keyword more complicated.

There has been users that have found it useful and it could be that @horsewithnoname1985 provides a PR.

I do agree that explaining this in the documentation is somewhat challenging. It might be worth while to consider to add a section to the library documentation and not only keyword documentation.

Fixed in #1176

Navigation Menu

Repository navigation

Allow arguments to be passed into JavaScript scripts #323

27 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Allow arguments to be passed into JavaScript scripts #323

Description

Activity

pekkaklarck commented on Aug 29, 2014

ombre42 commented on Sep 1, 2014

ombre42 commented on Sep 1, 2014

pekkaklarck commented on Sep 1, 2014

ombre42 commented on Sep 2, 2014

pekkaklarck commented on Sep 2, 2014

ombre42 commented on Sep 3, 2014

emanlove commented on Sep 3, 2014

emanlove commented on Sep 4, 2014

emanlove commented on Sep 4, 2014

pekkaklarck commented on Sep 5, 2014

ombre42 commented on Sep 6, 2014

just-be-dev commented on May 6, 2015

27 remaining items

arnewohletz commented on Jun 24, 2018

aaltat commented on Jun 25, 2018

aaltat commented on Jul 4, 2018

pekkaklarck commented on Jul 4, 2018

aaltat commented on Jul 4, 2018

pekkaklarck commented on Jul 4, 2018

aaltat commented on Jul 4, 2018

pekkaklarck commented on Jul 4, 2018

aaltat commented on Jul 4, 2018

aaltat commented on Sep 10, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions