Skip to content

Conversation

@divfor
Copy link
Contributor

@divfor divfor commented May 14, 2015

Code changes:
added 'Close Window And Select' to simplify two steps of 'Close Window' + 'Select Window'
added 'List Windows' to return all current handles;
enable 'Select Window' to always return the handle of window it is selecting from, and only return handle if 'self' as locator;
enriched 'Select Window' to select any opened window by a window handle;
enriched 'Select Window' to select new window by excluding a list of earlier window handles;

Background:
It is very useful to surface window handle to users that want to have the 'correct way' - comparing window handle sets, to switch window. If we do this way, the fix here for 'Close Window' is not required. instead, my new proposal is to update 'Select Window' and add 2 keywords:

Close Window And Select | locator=None

This will close current window and then switch to window if locator is given. locator can be name, title, url, handle, ..., the same as 'Select Window' requires.

${previous_window}= | Select Window | locator=None

This will switch to window matching locator and return previous window's handle. locator can be name, title, url, handle of target window, or a handle list to exclude in order to find new window. If locator is 'self', only return current window handle.

${window_list}= | List Windows

This will return a list of all opened window handles in same session not assuming its order.

Above changes should not impact existing usage of 'Close Window' and 'Select Window', and can offer the strict 'correct way' for using handle to switch window.

example 1:
A user should save main window handle in the beginning if required:

Open Browser 
${main_window}= | Select Window | self

A user can switch to correct window like below:

${all_old_windows}= | List Windows
Click Link | xxx | # or any other action that generates a child window
${parent_window}= | Select Window | ${all_old_windows}
Other Actions Against The Child Window
Close Window And Select | ${parent_window}

One more consideration:
should we also offer the additional simplified way to select the last-position indexed window as new window without the need to comparing with old window handles? The doc can prompt user to use strict way to select new window if the simplified way does not work for him/her. From my experiences the simplified way will be correct in 99% cases.

${parent_window}= | Select Window | new

divfor added 6 commits May 12, 2015 12:17
There are some special locators for searching target window:
string 'main' (default): select the main window;
string 'current': just return current window handle;
a single window handle: directly select window by handle;
string 'new': select new opened window assuming it is last-position
indexed (no iframe)
a past list of window handles: select new window by excluding past
window handle list
See 'Get Window Handles' to get the past list of window handles
add:
from selenium.common.exceptions import NoSuchWindowException
change current to self, fix exception proceeding
enable: select by handle, list all handles, select new by excluding old
handle list, close window and select
@zephraph
Copy link
Contributor

So I like most of this. I'm not so sure about the close and select keyword though. It's nice, but it seems redundant and we already have so many keywords.

@emanlove would you take a look at this?

@zephraph
Copy link
Contributor

Hey @divfor, do you think the Close and Select keyword is really necessary? I know it shorter, but I'm worried about keyword bloat.

@divfor
Copy link
Contributor Author

divfor commented May 23, 2015

it is not a must and can be written as a user keyword. I will remove it.

@divfor divfor mentioned this pull request May 24, 2015
@zephraph
Copy link
Contributor

Closing this in favor of #407.

@zephraph zephraph closed this May 24, 2015
@divfor
Copy link
Contributor Author

divfor commented May 24, 2015

I create a new PR #407 to resolve conflicts - as I tried to rebase my master but failed, so I re-clone rtomac/master

@pekkaklarck
Copy link
Member

I agree separate Close and Select keyword isn't really needed. Adding an option for Close to do that (e.g. select=None) might be nice, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants