Add a helper method to open a url in a new tab #182
Conversation
|
FluentLenium » FluentLenium #201 UNSTABLE |
|
Thank you for working on this. Feature seems to be good addition to FluentLenium functionalities. Have you run tests before you pushed the code to the VCS? Seems like one test is failing: org.fluentlenium.integration.FluentLeniumWaitTest.checkPolling please check what might be broken by your change if you want to have your change merged w/ current code. |
| if (baseUrl != null) { | ||
| URI uri = URI.create(url); | ||
| if (!uri.isAbsolute()) { | ||
| url = baseUrl + url; |
There was a problem hiding this comment.
I'm not sure if you want to always use base URL as a part of URL construction.
Java has gently mechanism to compose URL instead of String adding.
URL myURL = new URL("http://example.com/pages/");
URL page1URL = new URL(myURL, "page1.html");
URL page2URL = new URL(myURL, "page2.html");
Could you re-factor this method to do it in the better way?
There was a problem hiding this comment.
The URL construction logic comes from the existing method goTo.
|
FluentLenium » FluentLenium #211 SUCCESS |
|
I'm not sure but probably we are not supporting tabs switch, so if we are going to support new tab creation we should also support tabs switching are you able to extend your code to support such feature? |
|
Do you want this support in the same PR ? Do you thing to a The tricky thing is that A Let me know… |
|
I think it might be a few versions:
|
There could be more than one tab opened with the same URL Another point, ordered-aware switch will work only if all tabs are open with the method |
|
What is the concrete implementation of Set ? Some Set may have a defined iteration order (LinkedHashSet). |
| Set<String> initialTabs = getDriver().getWindowHandles(); | ||
| executeScript("window.open('" + url + "', '_blank');"); | ||
| Set<String> tabs = getDriver().getWindowHandles(); | ||
| tabs.removeAll(initialTabs); |
There was a problem hiding this comment.
The 3 previous lines requires thread synchronization
No description provided.