robotframework / OldSeleniumLibrary

Deprecated Selenium library for Robot Framework
Apache License 2.0
13 stars 3 forks source link

`Execute Javascript` keyword should support executing JS file directly #122

Closed spooning closed 9 years ago

spooning commented 9 years ago

Originally submitted to Google Code by rover.zh... on 10 Jun 2010

The keyword "Execute Javascript" now doesn't support execute JS file directly.So if it supports this it could be so fine! The content of the JS file could be:

<script type="text/javascript"> //the js codes.... </script>

spooning commented 9 years ago

Originally submitted to Google Code by Andreas.EbbertKarroum on 10 Jun 2010

Hi,

a nice idea. In the meantime, you could write your own keyword, which wrappes the execute javascript and loads the script file dynamically as described here:

http://www.codehouse.com/javascript/articles/external/

In a later revision of the SeliumLibrarey we could either extend the existing or write a new keyword.

<script type="text/javascript">

function dhtmlLoadScript(url) { var e = document.createElement("script"); e.src = url; e.type="text/javascript"; document.getElementsByTagName("head")[0].appendChild(e); }

onload = function() { dhtmlLoadScript("dhtml_way.js"); }

</script>

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 11 Jun 2010

I agree this enhancement is a good idea. For me it seems that this can be easily implemented like this:

--- a/src/SeleniumLibrary/javascript.py Thu May 27 11:57:21 2010 +0200 +++ b/src/SeleniumLibrary/javascript.py Fri Jun 11 13:59:39 2010 +0300 @⁠@⁠ -36,9 +36,18 @⁠@⁠ | Execute JavaScript | window.my_js_function('arg1', 'arg2') | """ js = ''.join([part.strip() for part in code])

Does anyone see problems with this approach? In addition to adding this code we should also:

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 1 Nov 2010

I think this looks good at first glance. Is there any way to do more fine-grained error handling / logging? I'd prefer some more info like "Checking whether argument '/tmp/foo.js' resolves to a file" plus the result. Maybe I'm being paranoid about usability here :) Opinions?

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 14 Nov 2010

Implemented, with tests and docs, in revision 98edb66a84. As documented, the keyword requires the path to be absolute in addition to matching an existing file. I don't think you could have valid JavaScript that looks like an absolute path, so more fine-grained error handling isn't probably needed.

Janne or Robert, could you review the changes and close this issue if you consider it done?

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 14 Nov 2010

In the end decided to add a bit more logging. That was done in revision c9467de794.

spooning commented 9 years ago

Originally submitted to Google Code by @yanne on 14 Nov 2010

Fixed on typo in the docs in r94b553fb483e

This seems to be done.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 15 Nov 2010

Looks good with the additional logging.