robotframework / OldSeleniumLibrary

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

`Confirm Action` keyword should return the message of javascipt confirmation #138

Closed spooning closed 9 years ago

spooning commented 9 years ago

Originally submitted to Google Code by xieyanbo on 26 Sep 2010

Confirm Action use selenium's get_confirmation method, but not return its value. That makes tester can not check confirmation message. Add a return syntax can fix this in javascript.py. Modify confirm_action: {{{ def confirm_action(self): self._selenium.get_confirmation() }}} as: {{{ def confirm_action(self): return self._selenium.get_confirmation() }}} will be OK.

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 26 Sep 2010

What does this method actually return? Should the return value be processed by the library somehow or is returning it as-is enough?

Returning the return value of the method in some format is definitely a good idea. In addition to changing the code, the documentation of the keyword needs to be updated (editing the docstring in the source code is enough) and its acceptance tests [1] should also be enhanced. xieyanbo, do you have time and interest to take care of them yourself and attach a patch? We'll obviously help you.

[1] https://code.google.com/p/robotframework-seleniumlibrary/source/browse/test/acceptance/keywords/javascript.txt

spooning commented 9 years ago

Originally submitted to Google Code by xieyanbo on 26 Sep 2010

Sure, I'll work on it.

spooning commented 9 years ago

Originally submitted to Google Code by xieyanbo on 2 Oct 2010

Here's the patch and all tests passed.

spooning commented 9 years ago

Originally submitted to Google Code by xieyanbo on 11 Nov 2010

Is this patch OK?

spooning commented 9 years ago

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

Sorry for not getting back earlier.

The patch looks fine, and will be incorporated in SeleniumLibrary 2.5, which will be released next week.

spooning commented 9 years ago

Originally submitted to Google Code by xieyanbo on 11 Nov 2010

Thanks, Janne.

spooning commented 9 years ago

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

Applied the patch in r2ba005f0055e