java: Making frame switching API more standard conformant. The standard claims that a driver must provide switching by index or element reference only. Switching by name and id should be implemented on the client side if we want to keep it in the API. In particular it is true for marionette

Author: barancev

java/client/src/org/openqa/selenium/remote/RemoteWebDriver.java

@@ -31,6 +31,8 @@
 import org.openqa.selenium.Dimension;
 import org.openqa.selenium.HasCapabilities;
 import org.openqa.selenium.JavascriptExecutor;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.NoSuchFrameException;
 import org.openqa.selenium.OutputType;
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.Point;
@@ -871,8 +873,17 @@ public WebDriver frame(int frameIndex) {
     }
 
     public WebDriver frame(String frameName) {
-      execute(DriverCommand.SWITCH_TO_FRAME, ImmutableMap.of("id", frameName));
-      return RemoteWebDriver.this;
+      String name = frameName.replaceAll("(['\"\\\\#.:;,!?+<>=~*^$|%&@`{}\\-/\\[\\]\\(\\)])", "\\\\$1");
+      List<WebElement> frameElements = RemoteWebDriver.this.findElements(
+          By.cssSelector("frame[name='" + name + "'],iframe[name='" + name + "']"));
+      if (frameElements.size() == 0) {
+        frameElements = RemoteWebDriver.this.findElements(
+            By.cssSelector("frame#" + name + ",iframe#" + name));
+      }
+      if (frameElements.size() == 0) {
+        throw new NoSuchFrameException("No frame element found by name or id " + frameName);
+      }
+      return frame(frameElements.get(0));
     }
 
     public WebDriver frame(WebElement frameElement) {

java/client/test/org/openqa/selenium/ClickTest.java

@@ -85,7 +85,6 @@ public void testCanClickOnAnAnchorAndNotReloadThePage() {
     assertEquals("Latch was reset", Boolean.TRUE, samePage);
   }
 
-  @Ignore(value = {MARIONETTE})
   @Test
   public void testCanClickOnALinkThatUpdatesAnotherFrame() {
     driver.switchTo().frame("source");
@@ -97,7 +96,6 @@ public void testCanClickOnALinkThatUpdatesAnotherFrame() {
   }
 
   @JavascriptEnabled
-  @Ignore(value = {MARIONETTE})
   @Test
   public void testElementsFoundByJsCanLoadUpdatesInAnotherFrame() {
     driver.switchTo().frame("source");
@@ -112,7 +110,6 @@ public void testElementsFoundByJsCanLoadUpdatesInAnotherFrame() {
   }
 
   @JavascriptEnabled
-  @Ignore(value = {MARIONETTE})
   @Test
   public void testJsLocatedElementsCanUpdateFramesIfFoundSomehowElse() {
     driver.switchTo().frame("source");

java/client/test/org/openqa/selenium/ElementEqualityTest.java

@@ -71,7 +71,6 @@ public void testSameElementLookedUpDifferentWaysUsingFindElementsShouldHaveSameH
   }
 
   @JavascriptEnabled
-  @Ignore(value = {MARIONETTE})
   @Test
   public void testAnElementFoundInADifferentFrameViaJsShouldHaveSameId() {
     driver.get(pages.missedJsReferencePage);

java/client/test/org/openqa/selenium/ElementFindingTest.java

@@ -739,7 +739,6 @@ public void testShouldNotBeAbleToLocateASingleElementOnABlankPage() {
     driver.findElement(By.id("nonExistantButton"));
   }
 
-  @Ignore(value = {MARIONETTE})
   @Test
   public void testAnElementFoundInADifferentFrameIsStale() {
     driver.get(pages.missedJsReferencePage);
@@ -755,7 +754,6 @@ public void testAnElementFoundInADifferentFrameIsStale() {
   }
 
   @JavascriptEnabled
-  @Ignore({MARIONETTE})
   @Test
   public void testAnElementFoundInADifferentFrameViaJsCanBeUsed() {
     driver.get(pages.missedJsReferencePage);

java/client/test/org/openqa/selenium/ExecutingJavascriptTest.java

@@ -424,7 +424,6 @@ public void testShouldBeAbleToPassInMoreThanOneArgument() {
 
   @JavascriptEnabled
   @Test
-  @Ignore({MARIONETTE})
   public void testShouldBeAbleToGrabTheBodyOfFrameOnceSwitchedTo() {
     driver.get(pages.richTextPage);
 

java/client/test/org/openqa/selenium/FrameSwitchingTest.java

@@ -113,7 +113,6 @@ public void testShouldBeAbleToSwitchToAnIframeByItsIndex() {
   }
 
   @Test
-  @Ignore(MARIONETTE)
   public void testShouldBeAbleToSwitchToAFrameByItsName() {
     driver.get(pages.framesetPage);
     driver.switchTo().frame("fourth");
@@ -122,7 +121,6 @@ public void testShouldBeAbleToSwitchToAFrameByItsName() {
   }
 
   @Test
-  @Ignore(MARIONETTE)
   public void testShouldBeAbleToSwitchToAnIframeByItsName() {
     driver.get(pages.iframePage);
     driver.switchTo().frame("iframe1-name");
@@ -131,23 +129,20 @@ public void testShouldBeAbleToSwitchToAnIframeByItsName() {
   }
 
   @Test
-  @Ignore(MARIONETTE)
   public void testShouldBeAbleToSwitchToAFrameByItsID() {
     driver.get(pages.framesetPage);
     driver.switchTo().frame("fifth");
     assertThat(driver.findElement(By.name("windowOne")).getText(), equalTo("Open new window"));
   }
 
   @Test
-  @Ignore(MARIONETTE)
   public void testShouldBeAbleToSwitchToAnIframeByItsID() {
     driver.get(pages.iframePage);
     driver.switchTo().frame("iframe1");
 
     assertThat(driver.findElement(By.name("id-name1")).getAttribute("value"), equalTo("name"));
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testShouldBeAbleToSwitchToFrameWithNameContainingDot() {
     driver.get(pages.framesetPage);
@@ -187,7 +182,6 @@ public void testShouldEnsureElementIsAFrameBeforeSwitching() {
     }
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testFrameSearchesShouldBeRelativeToTheCurrentlySelectedFrame() {
     driver.get(pages.framesetPage);
@@ -217,7 +211,6 @@ public void testFrameSearchesShouldBeRelativeToTheCurrentlySelectedFrame() {
     assertThat(driver.findElement(By.id("pageNumber")).getText(), equalTo("2"));
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testShouldSelectChildFramesByChainedCalls() {
     driver.get(pages.framesetPage);
@@ -226,7 +219,6 @@ public void testShouldSelectChildFramesByChainedCalls() {
     assertThat(driver.findElement(By.id("pageNumber")).getText(), equalTo("11"));
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testShouldThrowFrameNotFoundExceptionLookingUpSubFramesWithSuperFrameNames() {
     driver.get(pages.framesetPage);
@@ -365,7 +357,6 @@ public String getTextOfGreetingElement() {
     return wait.until(presenceOfElementLocated(By.id("greeting"))).getText();
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testShouldBeAbleToClickInAFrame() {
     driver.get(pages.framesetPage);
@@ -380,7 +371,6 @@ public void testShouldBeAbleToClickInAFrame() {
     assertThat(getTextOfGreetingElement(), equalTo("Success!"));
   }
 
-  @Ignore({MARIONETTE})
   @JavascriptEnabled
   @Test
   public void testShouldBeAbleToClickInAFrameThatRewritesTopWindowLocation() {
@@ -391,7 +381,6 @@ public void testShouldBeAbleToClickInAFrameThatRewritesTopWindowLocation() {
     wait.until(titleIs("Target page for issue 5237"));
   }
 
-  @Ignore({MARIONETTE})
   @Test
   public void testShouldBeAbleToClickInASubFrame() {
     driver.get(pages.framesetPage);
@@ -409,7 +398,6 @@ public void testShouldBeAbleToClickInASubFrame() {
     assertThat(driver.findElement(By.id("greeting")).getText(), equalTo("Success!"));
   }
 
-  @Ignore(MARIONETTE)
   @Test
   public void testShouldBeAbleToFindElementsInIframesByXPath() {
     driver.get(pages.iframePage);
@@ -440,7 +428,7 @@ public void testGetCurrentUrl() {
     assertThat(driver.getCurrentUrl(), equalTo(url));
   }
 
-  @Ignore(value = {PHANTOMJS, MARIONETTE})
+  @Ignore(value = {PHANTOMJS})
   @JavascriptEnabled
   @Test
   public void testShouldBeAbleToSwitchToTheTopIfTheFrameIsDeletedFromUnderUs() {
@@ -486,14 +474,12 @@ public void testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs() {
   }
 
   @Test
-  @Ignore(MARIONETTE)
   public void testShouldReturnWindowTitleInAFrameset() {
     driver.get(pages.framesetPage);
     driver.switchTo().frame("third");
     assertEquals("Unique title", driver.getTitle());
   }
 
-  @Ignore(MARIONETTE)
   @JavascriptEnabled
   @Test
   public void testJavaScriptShouldExecuteInTheContextOfTheCurrentFrame() {