Repository: robotframework/SeleniumLibrary
Pull request #1176 (2018-08-16 20:04:25)

JavaScript parser improvement

aaltat: Fixes #323 
aaltat: Need to take a look on few things:
1) Are error messages good.
2) Can `_separate_code_and_args` simplified. Now it has quite many moving parts. 
3) Proof read the new keyword docs. The error message says `marker` but doc says `statement`

Patch:
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 21f70296..af2ac900 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -3,7 +3,7 @@
 
 mockito >= 1.0.0
 robotstatuschecker
-approvaltests >= 0.2.3
+approvaltests >= 0.2.4
 
 # Include normal dependencies from requirements.txt. Makes it possible to use
 # requirements-dev.txt as a single requirement file in PyCharm and other IDEs.
diff --git a/src/SeleniumLibrary/keywords/alert.py b/src/SeleniumLibrary/keywords/alert.py
index 98022d49..99d1aa11 100644
--- a/src/SeleniumLibrary/keywords/alert.py
+++ b/src/SeleniumLibrary/keywords/alert.py
@@ -30,11 +30,12 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def input_text_into_prompt(self, text):
-        """Deprecated. Use `Input Text Into Alert` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Input Text Into Alert` instead.
 
         Types the given ``text`` into an input field in an alert.
         Leaves the alert open.
         """
+
         self.input_text_into_alert(text, self.LEAVE)
 
     @keyword
@@ -99,7 +100,7 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def choose_cancel_on_next_confirmation(self):
-        """Deprecated. Use `Handle Alert` directly instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Handle Alert` directly instead.
 
         In versions prior to SeleniumLibrary 3.0, the alert handling
         approach needed to be set separately before using the `Confirm
@@ -110,7 +111,7 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def choose_ok_on_next_confirmation(self):
-        """Deprecated. Use `Handle Alert` directly instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Handle Alert` directly instead.
 
         In versions prior to SeleniumLibrary 3.0, the alert handling
         approach needed to be set separately before using the `Confirm
@@ -121,7 +122,7 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def confirm_action(self):
-        """Deprecated. Use `Handle Alert` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Handle Alert` instead.
 
         By default accepts an alert, but this behavior can be altered
         with `Choose Cancel On Next Confirmation` and `Choose Ok On Next
@@ -135,7 +136,7 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def get_alert_message(self, dismiss=True):
-        """Deprecated. Use `Handle Alert` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Handle Alert` instead.
 
         Returns the message the alert has. Dismisses the alert by default
         (i.e. presses ``Cancel``) and setting ``dismiss`` to false leaves
@@ -150,7 +151,7 @@ class AlertKeywords(LibraryComponent):
 
     @keyword
     def dismiss_alert(self, accept=True):
-        """Deprecated. Use `Handle Alert` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Handle Alert` instead.
 
         Contrary to its name, this keyword accepts the alert by default
         (i.e. presses ``Ok``). ``accept`` can be set to a false value
diff --git a/src/SeleniumLibrary/keywords/cookie.py b/src/SeleniumLibrary/keywords/cookie.py
index 7fea010e..892110da 100644
--- a/src/SeleniumLibrary/keywords/cookie.py
+++ b/src/SeleniumLibrary/keywords/cookie.py
@@ -54,7 +54,7 @@ class CookieKeywords(LibraryComponent):
 
     @keyword
     def get_cookie_value(self, name):
-        """Deprecated. Use `Get Cookie` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Get Cookie` instead."""
         cookie = self.driver.get_cookie(name)
         if cookie is not None:
             return cookie['value']
diff --git a/src/SeleniumLibrary/keywords/element.py b/src/SeleniumLibrary/keywords/element.py
index 2bc6d518..691fa321 100644
--- a/src/SeleniumLibrary/keywords/element.py
+++ b/src/SeleniumLibrary/keywords/element.py
@@ -364,7 +364,7 @@ class ElementKeywords(LibraryComponent):
             raise AssertionError(message)
 
     @keyword
-    def get_element_attribute(self, locator, attribute=None):
+    def get_element_attribute(self, locator, attribute):
         """Returns value of ``attribute`` from element ``locator``.
 
         See the `Locating elements` section for details about the locator
@@ -373,14 +373,10 @@ class ElementKeywords(LibraryComponent):
         Example:
         | ${id}= | `Get Element Attribute` | css:h1 | id |
 
-        Passing attribute name as part of the ``locator`` is deprecated
-        since SeleniumLibrary 3.0. The explicit ``attribute`` argument
+        Passing attribute name as part of the ``locator`` was removed
+        in SeleniumLibrary 3.2. The explicit ``attribute`` argument
         should be used instead.
         """
-        if is_noney(attribute):
-            self.warn("Using 'Get Element Attribute' without explicit "
-                      "attribute is deprecated.")
-            locator, attribute = locator.rsplit('@', 1)
         return self.find_element(locator).get_attribute(attribute)
 
     @keyword
@@ -554,7 +550,7 @@ class ElementKeywords(LibraryComponent):
 
     @keyword
     def focus(self, locator):
-        """Deprecated. Use `Set Focus To Element` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Set Focus To Element` instead."""
         self.set_focus_to_element(locator)
 
     @keyword
@@ -693,7 +689,7 @@ return !element.dispatchEvent(evt);
 
     @keyword
     def simulate(self, locator, event):
-        """Deprecated. Use `Simulate Event` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Simulate Event` instead."""
         self.simulate_event(locator, event)
 
     @keyword
@@ -829,13 +825,13 @@ return !element.dispatchEvent(evt);
 
     @keyword
     def get_matching_xpath_count(self, xpath, return_str=True):
-        """Deprecated. Use `Get Element Count` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Get Element Count` instead."""
         count = self.get_element_count('xpath:' + xpath)
         return str(count) if is_truthy(return_str) else count
 
     @keyword
     def xpath_should_match_x_times(self, xpath, x, message=None, loglevel='INFO'):
-        """Deprecated, use `Page Should Contain Element` with ``limit`` argument instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Page Should Contain Element` with ``limit`` argument instead."""
         self.locator_should_match_x_times('xpath:'+xpath, x, message, loglevel)
 
     @keyword
diff --git a/src/SeleniumLibrary/keywords/frames.py b/src/SeleniumLibrary/keywords/frames.py
index e7a518b6..f29d0b4d 100644
--- a/src/SeleniumLibrary/keywords/frames.py
+++ b/src/SeleniumLibrary/keywords/frames.py
@@ -65,7 +65,7 @@ class FrameKeywords(LibraryComponent):
 
     @keyword
     def current_frame_contains(self, text, loglevel='INFO'):
-        """Deprecated. Use `Current Frame Should Contain` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Current Frame Should Contain` instead."""
         self.current_frame_should_contain(text, loglevel)
 
     @keyword
diff --git a/src/SeleniumLibrary/keywords/javascript.py b/src/SeleniumLibrary/keywords/javascript.py
index 27d4d24b..3ca2fdb8 100644
--- a/src/SeleniumLibrary/keywords/javascript.py
+++ b/src/SeleniumLibrary/keywords/javascript.py
@@ -15,21 +15,28 @@
 # limitations under the License.
 
 import os
+from collections import namedtuple
+
+from robot.utils import plural_or_not, seq2str
 
 from SeleniumLibrary.base import LibraryComponent, keyword
 
 
 class JavaScriptKeywords(LibraryComponent):
 
+    js_marker = 'JAVASCRIPT'
+    arg_marker = 'ARGUMENTS'
+
     @keyword
     def execute_javascript(self, *code):
-        """Executes the given JavaScript code.
+        """Executes the given JavaScript code with possible arguments.
 
-        ``code`` may contain multiple lines of code and may be divided into
-        multiple cells in the test data. In that case, the parts are
-        concatenated together without adding spaces.
+        ``code`` may be divided into multiple cells in the test data and
+        ``code`` may contain multiple lines of code and arguments. In that case,
+        the JavaScript code parts are concatenated together without adding
+        spaces and optional arguments are separated from ``code``.
 
-        If ``code`` is an absolute path to an existing file, the JavaScript
+        If ``code`` is a path to an existing file, the JavaScript
         to execute will be read from that file. Forward slashes work as
         a path separator on all operating systems.
 
@@ -42,19 +49,30 @@ class JavaScriptKeywords(LibraryComponent):
         This keyword returns whatever the executed JavaScript code returns.
         Return values are converted to the appropriate Python types.
 
+        Starting from SeleniumLibrary 3.2 it is possible to provide JavaScript
+        [https://seleniumhq.github.io/selenium/docs/api/py/webdriver_remote/selenium.webdriver.remote.webdriver.html#selenium.webdriver.remote.webdriver.WebDriver.execute_script|
+        arguments] as part of ``code`` argument. The JavaScript code and
+        arguments must be separated with `JAVASCRIPT` and `ARGUMENTS` markers
+        and must used exactly with this format. If the Javascript code is
+        first, then the `JAVASCRIPT` marker is optional. The order of
+        `JAVASCRIPT` and `ARGUMENTS` markers can swapped, but if `ARGUMENTS`
+        is first marker, then `JAVASCRIPT` marker is mandatory. It is only
+        allowed to use `JAVASCRIPT` and `ARGUMENTS` markers only one time in the
+        ``code`` argument.
+
         Examples:
         | `Execute JavaScript` | window.myFunc('arg1', 'arg2') |
         | `Execute JavaScript` | ${CURDIR}/js_to_execute.js    |
-        | ${sum} =             | `Execute JavaScript` | return 1 + 1; |
-        | `Should Be Equal`    | ${sum}               | ${2}          |
+        | `Execute JavaScript` | alert(arguments[0]); | ARGUMENTS | 123 |
+        | `Execute JavaScript` | ARGUMENTS | 123 | JAVASCRIPT | alert(arguments[0]); |
         """
-        js = self._get_javascript_to_execute(code)
-        self.info("Executing JavaScript:\n%s" % js)
-        return self.driver.execute_script(js)
+        js_code, js_args = self._get_javascript_to_execute(code)
+        self._js_logger('Executing JavaScript', js_code, js_args)
+        return self.driver.execute_script(js_code, *js_args)
 
     @keyword
     def execute_async_javascript(self, *code):
-        """Executes asynchronous JavaScript code.
+        """Executes asynchronous JavaScript code with possible arguments.
 
         Similar to `Execute Javascript` except that scripts executed with
         this keyword must explicitly signal they are finished by invoking the
@@ -64,6 +82,11 @@ class JavaScriptKeywords(LibraryComponent):
         Scripts must complete within the script timeout or this keyword will
         fail. See the `Timeouts` section for more information.
 
+        Starting from SeleniumLibrary 3.2 it is possible to provide JavaScript
+        [https://seleniumhq.github.io/selenium/docs/api/py/webdriver_remote/selenium.webdriver.remote.webdriver.html#selenium.webdriver.remote.webdriver.WebDriver.execute_async_script|
+        arguments] as part of ``code`` argument. See `Execute Javascript` for
+        more details.
+
         Examples:
         | `Execute Async JavaScript` | var callback = arguments[arguments.length - 1]; window.setTimeout(callback, 2000); |
         | `Execute Async JavaScript` | ${CURDIR}/async_js_to_execute.js |
@@ -73,19 +96,72 @@ class JavaScriptKeywords(LibraryComponent):
         | ...         | window.setTimeout(answer, 2000);                |
         | `Should Be Equal` | ${result} | text |
         """
-        js = self._get_javascript_to_execute(code)
-        self.info("Executing Asynchronous JavaScript:\n%s" % js)
-        return self.driver.execute_async_script(js)
-
-    def _get_javascript_to_execute(self, lines):
-        code = ''.join(lines)
-        path = code.replace('/', os.sep)
-        if os.path.isabs(path) and os.path.isfile(path):
-            code = self._read_javascript_from_file(path)
-        return code
+        js_code, js_args = self._get_javascript_to_execute(code)
+        self._js_logger('Executing Asynchronous JavaScript', js_code, js_args)
+        return self.driver.execute_async_script(js_code, *js_args)
+
+    def _js_logger(self, base, code, args):
+        message = '%s:\n%s\n' % (base, code)
+        if args:
+            message = ('%sBy using argument%s:\n%s'
+                       % (message, plural_or_not(args), seq2str(args)))
+        else:
+            message = '%sWithout any arguments.' % message
+        self.info(message)
+
+    def _get_javascript_to_execute(self, code):
+        js_code, js_args = self._separate_code_and_args(code)
+        if not js_code:
+            raise ValueError('JavaScript code was not found from code argument.')
+        js_code = ''.join(js_code)
+        path = js_code.replace('/', os.sep)
+        if os.path.isfile(path):
+            js_code = self._read_javascript_from_file(path)
+        return js_code, js_args
+
+    def _separate_code_and_args(self, code):
+        code = list(code)
+        self._check_marker_error(code)
+        index = self._get_marker_index(code)
+        if self.arg_marker not in code:
+            return code[index.js + 1:], []
+        if self.js_marker not in code:
+            return code[0:index.arg], code[index.arg + 1:]
+        else:
+            if index.js == 0:
+                return code[index.js + 1:index.arg], code[index.arg + 1:]
+            else:
+                return code[index.js + 1:], code[index.arg + 1:index.js]
+
+    def _check_marker_error(self, code):
+        if not code:
+            raise ValueError('There must be at least one argument defined.')
+        message = None
+        template = '%s marker was found two times in the code.'
+        if code.count(self.js_marker) > 1:
+            message = template % self.js_marker
+        if code.count(self.arg_marker) > 1:
+            message = template % self.arg_marker
+        index = self._get_marker_index(code)
+        if index.js > 0 and index.arg != 0:
+            message = template % self.js_marker
+        if message:
+            raise ValueError(message)
+
+    def _get_marker_index(self, code):
+        Index = namedtuple('Index', 'js arg')
+        if self.js_marker in code:
+            js = code.index(self.js_marker)
+        else:
+            js = -1
+        if self.arg_marker in code:
+            arg = code.index(self.arg_marker)
+        else:
+            arg = -1
+        return Index(js=js, arg=arg)
 
     def _read_javascript_from_file(self, path):
         self.info('Reading JavaScript from file <a href="file://%s">%s</a>.'
-                  .format(path.replace(os.sep, '/'), path), html=True)
+                  % (path.replace(os.sep, '/'), path), html=True)
         with open(path) as file:
             return file.read().strip()
diff --git a/src/SeleniumLibrary/keywords/screenshot.py b/src/SeleniumLibrary/keywords/screenshot.py
index 2446fb1b..4bffe2ad 100644
--- a/src/SeleniumLibrary/keywords/screenshot.py
+++ b/src/SeleniumLibrary/keywords/screenshot.py
@@ -25,7 +25,7 @@ from SeleniumLibrary.utils import is_noney
 class ScreenshotKeywords(LibraryComponent):
 
     @keyword
-    def set_screenshot_directory(self, path, persist='DEPRECATED'):
+    def set_screenshot_directory(self, path):
         """Sets the directory for captured screenshots.
 
         ``path`` argument specifies the absolute path to a directory where
@@ -40,17 +40,14 @@ class ScreenshotKeywords(LibraryComponent):
         The previous value is returned and can be used to restore
         the original value later if needed.
 
-        Deprecating ``persist`` and returning the previous value are new
-        in SeleniumLibrary 3.0.
+        Returning the previous value is new in SeleniumLibrary 3.0.
+        The persist argument was removed in SeleniumLibrary 3.2.
         """
         if is_noney(path):
             path = None
         else:
             path = os.path.abspath(path)
             self._create_directory(path)
-        if persist != 'DEPRECATED':
-            self.warn("'persist' argument to 'Set Screenshot Directory' "
-                      "keyword is deprecated and has no effect.")
         previous = self.ctx.screenshot_root_directory
         self.ctx.screenshot_root_directory = path
         return previous
diff --git a/src/SeleniumLibrary/keywords/selectelement.py b/src/SeleniumLibrary/keywords/selectelement.py
index 81672b67..7c734c40 100644
--- a/src/SeleniumLibrary/keywords/selectelement.py
+++ b/src/SeleniumLibrary/keywords/selectelement.py
@@ -191,7 +191,7 @@ class SelectElementKeywords(LibraryComponent):
 
     @keyword
     def select_from_list(self, locator, *options):
-        """Deprecated. Use `Select From List By Label/Value/Index` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Select From List By Label/Value/Index` instead.
 
         This keyword selects options based on labels or values, which makes
         it very complicated and slow. It has been deprecated in
@@ -307,7 +307,7 @@ class SelectElementKeywords(LibraryComponent):
 
     @keyword
     def unselect_from_list(self, locator, *items):
-        """Deprecated. Use `Unselect From List By Label/Value/Index` instead.
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Unselect From List By Label/Value/Index` instead.
 
         This keyword unselects options based on labels or values, which makes
         it very complicated and slow. It has been deprecated in
diff --git a/src/SeleniumLibrary/keywords/window.py b/src/SeleniumLibrary/keywords/window.py
index 90d22add..36e343ea 100644
--- a/src/SeleniumLibrary/keywords/window.py
+++ b/src/SeleniumLibrary/keywords/window.py
@@ -127,7 +127,7 @@ class WindowKeywords(LibraryComponent):
 
     @keyword
     def list_windows(self):
-        """Deprecated. Use `Get Window Handles` instead."""
+        """*DEPRECATED in SeleniumLibrary 3.2.* Use `Get Window Handles` instead."""
         return self.get_window_handles()
 
     @keyword


Test patch:
diff --git a/atest/acceptance/keywords/async_javascript.robot b/atest/acceptance/keywords/async_javascript.robot
index 69d78fde..2a9f8fcb 100644
--- a/atest/acceptance/keywords/async_javascript.robot
+++ b/atest/acceptance/keywords/async_javascript.robot
@@ -6,9 +6,19 @@ Resource          ../resource.robot
 
 *** Test Cases ***
 Should Not Timeout If Callback Invoked Immediately
-    ${result} =    Execute Async Javascript    arguments[arguments.length - 1](123);
+    ${result} =    Execute Async Javascript
+    ...    JAVASCRIPT
+    ...    arguments[arguments.length - 1](123);
     Should Be Equal    ${result}    ${123}
 
+Execute Async Javascript With ARGUMENTS and JAVASCRIPT Marker
+    Execute Async Javascript
+    ...  ARGUMENTS
+    ...  123
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    Alert Should Be Present    123    timeout=10 s
+
 Should Be Able To Return Javascript Primitives From Async Scripts Neither None Nor Undefined
     ${result} =    Execute Async Javascript    arguments[arguments.length - 1](123);
     Should Be Equal    ${result}    ${123}
diff --git a/atest/acceptance/keywords/elements.robot b/atest/acceptance/keywords/elements.robot
index 6eddf772..8089d23c 100644
--- a/atest/acceptance/keywords/elements.robot
+++ b/atest/acceptance/keywords/elements.robot
@@ -40,22 +40,19 @@ Assign Id To Element
     Page Should Not Contain Element    my id
 
 Get Element Attribute
-    ${id}=    Get Element Attribute    link=Link with id@id
+    ${id}=    Get Element Attribute    link:Link with id    id
     Should Be Equal    ${id}    some_id
-    ${id}=    Get Element Attribute    dom=document.getElementsByTagName('a')[3]@id
+    ${id}=    Get Element Attribute    dom:document.getElementsByTagName('a')[3]    id
     Should Be Equal    ${id}    some_id
-    ${class}=    Get Element Attribute    second_div@class
+    ${class}=    Get Element Attribute    second_div    class
     Should Be Equal    ${class}    Second Class
     ${id}=    Get Element Attribute    link=Link with id    id
     Should Be Equal    ${id}    some_id
-    ${element_by_dom}=    Get Webelement    dom=document.getElementsByTagName('a')[3]
-    ${id}=    Get Element Attribute    ${element_by_dom}    id
-    Should Be Equal    ${id}    some_id
     ${second_div}=    Get Webelement    second_div
     ${class}=    Get Element Attribute    ${second_div}    class
     Should Be Equal    ${class}    Second Class
 
-Get Element Attribute Value Should Be Should Be Succesfull 
+Get Element Attribute Value Should Be Should Be Succesfull
     Element Attribute Value Should Be  link=Absolute external link  href  http://www.google.com/
 
 Get Element Attribute And Element Attribute Value Should Be Should have same results
diff --git a/atest/acceptance/keywords/javascript.robot b/atest/acceptance/keywords/javascript.robot
index 3e248ecd..03e71da4 100644
--- a/atest/acceptance/keywords/javascript.robot
+++ b/atest/acceptance/keywords/javascript.robot
@@ -20,18 +20,63 @@ Mouse Down On Link
     Mouse Up    link_mousedown
 
 Execute Javascript
-    [Documentation]    LOG 2 Executing JavaScript:
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
     ...    window.add_content('button_target', 'Inserted directly')
+    ...    Without any arguments.
     Execute Javascript    window.add_content('button_target', 'Inserted directly')
     Page Should Contain    Inserted directly
 
+Execute Javascript With ARGUMENTS and JAVASCRIPT Marker
+    Execute Javascript
+    ...  ARGUMENTS
+    ...  123
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    Alert Should Be Present    123    timeout=10 s
+
+Execute Javascript With JAVASCRIPT and ARGUMENTS Marker
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
+    ...    alert(arguments[0]);
+    ...    By using argument:
+    ...    '123'
+    Execute Javascript
+    ...  JAVASCRIPT
+    ...  alert(arguments[0]);
+    ...  ARGUMENTS
+    ...  123
+    Alert Should Be Present    123    timeout=10 s
+
+Execute Javascript With ARGUMENTS Marker Only
+    [Documentation]
+    ...    LOG 2 Executing JavaScript:
+    ...    alert(arguments[0]);
+    ...    By using arguments:
+    ...    '123' and '0987'
+    Execute Javascript
+    ...  alert(arguments[0]);
+    ...  ARGUMENTS
+    ...  123
+    ...  0987
+    Alert Should Be Present    123    timeout=10 s
+
 Execute Javascript from File
-    [Documentation]    LOG 2:1 REGEXP: Reading JavaScript from file .*
+    [Documentation]
+    ...    LOG 2:1 REGEXP: Reading JavaScript from file .*executed_by_execute_javascript.*
     ...    LOG 2:2 Executing JavaScript:
     ...    window.add_content('button_target', 'Inserted via file')
+    ...    Without any arguments.
     Execute Javascript    ${CURDIR}/executed_by_execute_javascript.js
     Page Should Contain    Inserted via file
 
+Execute Javascript from File With ARGUMENTS Marker
+    Execute Javascript
+    ...    ${CURDIR}/javascript_alert.js
+    ...    ARGUMENTS
+    ...    123
+    Alert Should Be Present    123    timeout=10 s
+
 Open Context Menu
     [Tags]    Known Issue Safari
     Go To Page "javascript/context_menu.html"
diff --git a/atest/acceptance/keywords/javascript_alert.js b/atest/acceptance/keywords/javascript_alert.js
new file mode 100644
index 00000000..b42d042c
--- /dev/null
+++ b/atest/acceptance/keywords/javascript_alert.js
@@ -0,0 +1,1 @@
+alert(arguments[0]);
diff --git a/atest/acceptance/keywords/lists.robot b/atest/acceptance/keywords/lists.robot
index af405931..d7cf8e5e 100644
--- a/atest/acceptance/keywords/lists.robot
+++ b/atest/acceptance/keywords/lists.robot
@@ -74,7 +74,6 @@ List Selection Should Be When List Does Not Exist
     ...    List Selection Should Be    nonexisting    whatever
 
 UnSelect Single Value From List
-    [Documentation]    LOG 2.1 Unselecting option(s) 'Email' from list 'possible_channels'.
     Unselect and Verify Selection    possible_channels    Email    phone
     # Unselecting already unselected option has no effect
     Unselect and Verify Selection    possible_channels    Email    phone
@@ -84,7 +83,6 @@ UnSelect Single Value From List
     ...    Unselect From List    preferred_channel
 
 Unselect all options using Unselect From List
-    [Documentation]    LOG 2 Unselecting all options from list 'possible_channels'.
     Unselect From List    possible_channels
     List Should Have No Selections    possible_channels
 
@@ -99,12 +97,11 @@ Unselect All From List
     List Should Have No Selections    interests
 
 Select From Single Selection List
-    [Documentation]    LOG 2.1 Selecting option(s) 'Email' from list 'preferred_channel'.
     Select And verify selection    preferred_channel    Email    Email
     Select And verify selection    preferred_channel    Email    Email
     Select And verify selection    preferred_channel    directmail    Direct mail
     Select From List    preferred_channel    Telephone
-    # do something else... anything to ensure the list is really set as the next keyword will pass 
+    # do something else... anything to ensure the list is really set as the next keyword will pass
     # if list item is highlighted but not selected
     Unselect from List    possible_channels
     List Selection Should Be    preferred_channel    Telephone
@@ -135,12 +132,10 @@ Select Non-Existing Item From Multi-Selection List
     ...    Select From List    possible_channels    Tin Can Phone    Email    Smoke Signals
 
 Unselect Non-Existing Item From List
-    [Documentation]    LOG 3 Unselecting option(s) 'Tin Can Phone, Smoke Signals, Email' from list 'possible_channels'.
     Unselect From List    possible_channels    Tin Can Phone    Smoke Signals
     Unselect From List    possible_channels    Tin Can Phone    Smoke Signals    Email
 
 Select From Multiselect List
-    [Documentation]    LOG 6 Selecting option(s) 'Direct mail, phone' from list 'possible_channels'.
     Select And verify selection    possible_channels    Email    Email    Telephone
     Select And verify selection    possible_channels    email    email    phone
     Select And verify selection    possible_channels    Direct mail    Direct mail    Email    Telephone
diff --git a/atest/acceptance/windows.robot b/atest/acceptance/windows.robot
index e2d0502e..852fc23e 100644
--- a/atest/acceptance/windows.robot
+++ b/atest/acceptance/windows.robot
@@ -60,10 +60,10 @@ Get and Set Window Size
 
 Set Window Size using strings
     [Tags]  Known Issue Internet Explorer    Known Issue Safari
-    Set Window Size    456    654
+    Set Window Size    600    800
     ${width}    ${height}=    Get Window Size
-    Should Be Equal    ${width}    ${456}
-    Should Be Equal    ${height}    ${654}
+    Should Be Equal    ${width}    ${600}
+    Should Be Equal    ${height}    ${800}
 
 Get and Set Window Position
     [Tags]  Known Issue Chrome    Known Issue Safari
diff --git a/utest/test/keywords/approvaltests_config.json b/utest/test/keywords/approvaltests_config.json
index 1ceea7eb..550e6643 100644
--- a/utest/test/keywords/approvaltests_config.json
+++ b/utest/test/keywords/approvaltests_config.json
@@ -1,3 +1,3 @@
 {
   "subdirectory": "approved_files"
-}
\ No newline at end of file
+}
diff --git a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_check_marker_error.approved.txt b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_check_marker_error.approved.txt
new file mode 100644
index 00000000..b8a0d70f
--- /dev/null
+++ b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_check_marker_error.approved.txt
@@ -0,0 +1,24 @@
+error
+
+0) There must be at least one argument defined.
+1) ARGUMENTS marker was found two times in the code.
+2) JAVASCRIPT marker was found two times in the code.
+3) ARGUMENTS marker was found two times in the code.
+4) JAVASCRIPT marker was found two times in the code.
+5) ARGUMENTS marker was found two times in the code.
+6) JAVASCRIPT marker was found two times in the code.
+7) There must be at least one argument defined.
+8) None
+9) None
+10) None
+11) None
+12) None
+13) None
+14) None
+15) None
+16) None
+17) None
+18) None
+19) None
+20) None
+21) None
diff --git a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript.approved.txt b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript.approved.txt
index 140a65a1..7f2c07d4 100644
--- a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript.approved.txt
+++ b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript.approved.txt
@@ -1,1 +1,1 @@
-code here
\ No newline at end of file
+codehere + []
\ No newline at end of file
diff --git a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript_no_code.approved.txt b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript_no_code.approved.txt
new file mode 100644
index 00000000..5936febd
--- /dev/null
+++ b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_get_javascript_no_code.approved.txt
@@ -0,0 +1,1 @@
+JavaScript code was not found from code argument.
\ No newline at end of file
diff --git a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_indexing.approved.txt b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_indexing.approved.txt
new file mode 100644
index 00000000..39d0c38c
--- /dev/null
+++ b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_indexing.approved.txt
@@ -0,0 +1,17 @@
+index
+
+0) Index(js=-1, arg=-1)
+1) Index(js=-1, arg=-1)
+2) Index(js=-1, arg=-1)
+3) Index(js=0, arg=-1)
+4) Index(js=-1, arg=2)
+5) Index(js=-1, arg=-1)
+6) Index(js=-1, arg=-1)
+7) Index(js=0, arg=3)
+8) Index(js=0, arg=-1)
+9) Index(js=1, arg=0)
+10) Index(js=0, arg=3)
+11) Index(js=3, arg=0)
+12) Index(js=-1, arg=-1)
+13) Index(js=-1, arg=-1)
+14) Index(js=0, arg=-1)
diff --git a/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_separate_code_and_args.approved.txt b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_separate_code_and_args.approved.txt
new file mode 100644
index 00000000..59112862
--- /dev/null
+++ b/utest/test/keywords/approved_files/JavaScriptKeywordsTest.test_separate_code_and_args.approved.txt
@@ -0,0 +1,17 @@
+code and args
+
+0) There must be at least one argument defined.
+1) (['code1'], [])
+2) (['code1', 'code2'], [])
+3) (['code1', 'code2'], [])
+4) (['code1', 'code2'], ['arg1', 'arg2'])
+5) (['code1', 'code2', 'arguments', 'arg1', 'arg2'], [])
+6) (['javascript', 'code1', 'code2'], [])
+7) (['code1', 'code2'], [])
+8) (['code1', 'code2', 'argUMENTs'], [])
+9) (['code1', 'code2'], [])
+10) (['code1', 'code2'], ['arg1', 'arg2'])
+11) (['code1', 'code2'], ['arg1', 'arg2'])
+12) (['aRGUMENTS', 'arg1', 'arg2', 'jAVASCRIPT', 'code1', 'code2'], [])
+13) (['JAVASCRIPTCODE', 'code1', 'code2'], [])
+14) (['code1', 'code2', 'ARGUMENTS ARG2', 'arg3'], [])
diff --git a/utest/test/keywords/test_javascript.py b/utest/test/keywords/test_javascript.py
index 32bac4ed..c53d7b6d 100644
--- a/utest/test/keywords/test_javascript.py
+++ b/utest/test/keywords/test_javascript.py
@@ -3,7 +3,7 @@ import unittest
 
 from SeleniumLibrary.utils.platform import JYTHON
 try:
-    from approvaltests.approvals import verify
+    from approvaltests.approvals import verify, verify_all
     from approvaltests.reporters.generic_diff_reporter_factory import GenericDiffReporterFactory
 except ImportError:
     if JYTHON:
@@ -20,7 +20,21 @@ class JavaScriptKeywordsTest(unittest.TestCase):
     @classmethod
     @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
     def setUpClass(cls):
-        cls.javascript = JavaScriptKeywords(None)
+        cls.code_examples = [(),
+            ('code1',), ('code1', 'code2'),
+            ('JAVASCRIPT', 'code1', 'code2'),
+            ('code1', 'code2', 'ARGUMENTS', 'arg1', 'arg2'),
+            ('code1', 'code2', 'arguments', 'arg1', 'arg2'),
+            ('javascript', 'code1', 'code2'),
+            ('JAVASCRIPT', 'code1', 'code2', 'ARGUMENTS'),
+            ('JAVASCRIPT', 'code1', 'code2', 'argUMENTs'),
+            ('ARGUMENTS', 'JAVASCRIPT', 'code1', 'code2'),
+            ('JAVASCRIPT', 'code1', 'code2', 'ARGUMENTS', 'arg1', 'arg2'),
+            ('ARGUMENTS', 'arg1', 'arg2', 'JAVASCRIPT', 'code1', 'code2'),
+            ('aRGUMENTS', 'arg1', 'arg2', 'jAVASCRIPT', 'code1', 'code2'),
+            ('JAVASCRIPTCODE', 'code1', 'code2'),
+            ('JAVASCRIPT', 'code1', 'code2', 'ARGUMENTS ARG2', 'arg3')]
+        cls.js = JavaScriptKeywords(None)
         path = os.path.dirname(__file__)
         reporter_json = os.path.abspath(os.path.join(path, '..', 'approvals_reporters.json'))
         factory = GenericDiffReporterFactory()
@@ -29,5 +43,59 @@ class JavaScriptKeywordsTest(unittest.TestCase):
 
     @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
     def test_get_javascript(self):
-        code = self.javascript._get_javascript_to_execute('code here')
-        verify(code, self.reporter)
\ No newline at end of file
+        code, args = self.js._get_javascript_to_execute(('code', 'here'))
+        result = '%s + %s' % (code, args)
+        verify(result, self.reporter)
+
+    @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
+    def test_get_javascript_no_code(self):
+        code = ('ARGUMENTS', 'arg1', 'arg1')
+        try:
+            self.js._get_javascript_to_execute(code)
+        except Exception as error:
+            result = str(error)
+        verify(result, self.reporter)
+
+    @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
+    def test_separate_code_and_args(self):
+        all_results = []
+        for code in self.code_examples:
+            all_results.append(self.js_reporter(code))
+        verify_all('code and args', all_results, reporter=self.reporter)
+
+    @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
+    def test_indexing(self):
+        all_results = []
+        for code in self.code_examples:
+            all_results.append(self.js._get_marker_index(code))
+        verify_all('index', all_results, reporter=self.reporter)
+
+    @unittest.skipIf(JYTHON, 'ApprovalTest does not work with Jython')
+    def test_check_marker_error(self):
+        examples = [
+            (),
+            ('ARGUMENTS', 'arg1', 'ARGUMENTS', 'arg1', 'JAVASCRIPT',
+             'code1', 'JAVASCRIPT', 'code2'),
+            ('JAVASCRIPT', 'code1', 'JAVASCRIPT', 'code2'),
+            ('JAVASCRIPT', 'code1', 'ARGUMENTS', 'arg1', 'ARGUMENTS', 'arg1'),
+            ('code1', 'JAVASCRIPT', 'code1' 'ARGUMENTS', 'arg1',),
+            ('ARGUMENTS', 'arg1', 'ARGUMENTS', 'arg1', 'JAVASCRIPT', 'code1'),
+            ('aRGUMENtS', 'arg1', 'arg2', 'JAVASCRIPT', 'code1', 'code2'),
+        ]
+        examples = examples + self.code_examples
+        all_results = []
+        for code in examples:
+            all_results.append(self.js_marker_error(code))
+        verify_all('error', all_results, reporter=self.reporter)
+
+    def js_marker_error(self, code):
+        try:
+            return self.js._check_marker_error(code)
+        except Exception as error:
+            return error
+
+    def js_reporter(self, code):
+        try:
+            return self.js._separate_code_and_args(code)
+        except Exception as error:
+            return error
diff --git a/utest/test/keywords/test_keyword_arguments_element.py b/utest/test/keywords/test_keyword_arguments_element.py
index 9d460563..8e5a3cd1 100644
--- a/utest/test/keywords/test_keyword_arguments_element.py
+++ b/utest/test/keywords/test_keyword_arguments_element.py
@@ -35,20 +35,6 @@ class KeywordArgumentsElementTest(unittest.TestCase):
         with self.assertRaisesRegexp(AssertionError, 'foobar'):
             self.element.element_text_should_be(locator, 'not text', 'foobar')
 
-    def test_get_element_attribute(self):
-        locator = '//div'
-        attrib = 'id'
-        element = mock()
-        when(self.element).find_element(locator).thenReturn(element)
-        when(element).get_attribute(attrib).thenReturn('value')
-        value = self.element.get_element_attribute(locator, attrib)
-        self.assertEqual(value, 'value')
-
-        when(logger).warn("Using 'Get Element Attribute' without explicit "
-                          "attribute is deprecated.", False).thenReturn(None)
-        value = self.element.get_element_attribute('//div@id', 'None')
-        self.assertEqual(value, 'value')
-
     def test_get_matching_xpath_count(self):
         locator = '//div'
         when(self.element).find_elements('xpath:' + locator).thenReturn([])