Use selenium select class

[emanlove]

Switch over to using Selenium's Select() class instead of duplicating code within robotframework-selenium2library.

[emanlove]

@j1z0 or @rtomac : Could either of you review this? I prefer to have a pair of second eyes look over it and someone else merge my work.

[j1z0]

@emanlove

Looks good! A couple of comments though...
First let me qualify that when using "standard" webdriver I don't see any issues with this code. But I'm concerned about performance when using the remote webdriver. It has been my experience that when using the remote webdriver you want to reduce the number of calls to webdriver as each call incurs a round trip costs, and if (like me) your using something like SauceLabs (in the US) from a server in Belgium, that round trip cost can be significant. So at least from that point of view...

1. get_selected_list_value and get_selected_list_label both traverse the entire list, even if the selected value / label you want is the first item. Which means the minimum number of round trips is the length of the list. I would prefer for this to use the webdriver.first_selected_option() instead of the webdriver.all_selected_options() as I believe that should be more performant.
2. This is also somewhat of a change in functionality request driven by a desire for higher performance. The function unselect_from_list does a whole lot of work (webdriver calls) so the user can pass in the visible text or the value and then it unselects everything via index. However the function 'select_from_list' only works on visible text. Thus why not make unselecte_from_list also only work on visible text and then you just have to call webdriver.deselect_by_visible_text() we could then add additional functions for select / unselect by index and select / unselect by value. This would also mirror the webdriver API, which as discussed in previous issues, may make it harder for selenium 1 users to upgrade / update but IMHO is a better way moving forward...

what do you think?

[emanlove]

@emanlove

Looks good! A couple of comments though...
First let me qualify that when using "standard" webdriver I don't see any issues with this code. But I'm concerned about performance when using the remote webdriver. It has been my experience that when using the remote webdriver you want to reduce the number of calls to webdriver as each call incurs a round trip costs, and if (like me) your using something like SauceLabs (in the US) from a server in Belgium, that round trip cost can be significant. So at least from that point of view...

1. get_selected_list_value and get_selected_list_label both traverse the entire list, even if the selected value / label you want is the first item. Which means the minimum number of round trips is the length of the list. I would prefer for this to use the webdriver.first_selected_option() instead of the webdriver.all_selected_options() as I believe that should be more performant.
2. This is also somewhat of a change in functionality request driven by a desire for higher performance. The function unselect_from_list does a whole lot of work (webdriver calls) so the user can pass in the visible text or the value and then it unselects everything via index. However the function 'select_from_list' only works on visible text. Thus why not make unselecte_from_list also only work on visible text and then you just have to call webdriver.deselect_by_visible_text() we could then add additional functions for select / unselect by index and select / unselect by value. This would also mirror the webdriver API, which as discussed in previous issues, may make it harder for selenium 1 users to upgrade / update but IMHO is a better way moving forward...

what do you think?

@j1z0,

I think you have made some real good points about performance; don't
merge yet as I will take a look at these issues.

Ed

[emanlove]

@j1z0: Could you relook at these recent changes by @schminitz as I think they might address some of your concerns with performance. Thanks!