[java] only allow enabled select lists for Select class

titusfortner · titusfortner · commit 25b30ffa6657 · 2022-09-11T18:58:56.000-05:00
diff --git a/java/src/org/openqa/selenium/support/ui/Select.java b/java/src/org/openqa/selenium/support/ui/Select.java
@@ -45,10 +45,14 @@ public class Select implements ISelect, WrapsElement {
   public Select(WebElement element) {
     String tagName = element.getTagName();
 
-    if (null == tagName || !"select".equals(tagName.toLowerCase())) {
+    if (!"select".equalsIgnoreCase(tagName)) {
       throw new UnexpectedTagNameException("select", tagName);
     }
 
+    if (!element.isEnabled()) {
+      throw new UnsupportedOperationException("Select element is disabled and may not be used.");
+    }
+
     this.element = element;
 
     String value = element.getDomAttribute("multiple");
@@ -109,8 +113,6 @@ public WebElement getFirstSelectedOption() {
    */
   @Override
   public void selectByVisibleText(String text) {
-    assertSelectIsEnabled();
-
     // try to find the option via XPATH ...
     List<WebElement> options =
       element.findElements(By.xpath(".//option[normalize-space(.) = " + Quotes.escape(text) + "]"));
@@ -175,7 +177,6 @@ private String getLongestSubstringWithoutSpace(String s) {
    */
   @Override
   public void selectByIndex(int index) {
-    assertSelectIsEnabled();
     setSelectedByIndex(index, true);
   }
 
@@ -190,7 +191,6 @@ public void selectByIndex(int index) {
    */
   @Override
   public void selectByValue(String value) {
-    assertSelectIsEnabled();
     for (WebElement option : findOptionsByValue(value)) {
       setSelected(option, true);
       if (!isMultiple()) {
@@ -327,12 +327,6 @@ private void assertOptionIsEnabled(WebElement option, boolean select) {
     }
   }
 
-  private void assertSelectIsEnabled() {
-    if (!element.isEnabled()) {
-      throw new UnsupportedOperationException("You may not select an option in disabled select");
-    }
-  }
-
   @Override
   public boolean equals(Object o) {
     if (!(o instanceof Select)) {
diff --git a/java/test/org/openqa/selenium/support/ui/SelectElementTest.java b/java/test/org/openqa/selenium/support/ui/SelectElementTest.java
@@ -43,6 +43,13 @@ public void shouldThrowAnExceptionIfTheElementIsNotASelectElement() {
         .isThrownBy(() -> new Select(selectElement));
   }
 
+  @Test
+  public void shouldThrowAnExceptionIfTheElementIsDisabled() {
+    WebElement selectElement = driver.findElement(By.name("no-select"));
+    assertThatExceptionOfType(UnsupportedOperationException.class)
+      .isThrownBy(() -> new Select(selectElement));
+  }
+
   @Test
   public void shouldIndicateThatASelectCanSupportMultipleOptions() {
     WebElement selectElement = driver.findElement(By.name("multi"));
@@ -85,7 +92,6 @@ public void shouldReturnAllOptionsWhenAsked() {
 
     assertThat(select.getOptions()).extracting(WebElement::getText)
         .containsExactly("One", "Two", "Four", "Still learning how to count, apparently");
-
   }
 
   @Test
@@ -149,7 +155,16 @@ public void shouldThrowExceptionOnSelectByVisibleTextIfOptionDoesNotExist() {
     Select select = new Select(selectElement);
 
     assertThatExceptionOfType(NoSuchElementException.class)
-        .isThrownBy(() -> select.selectByVisibleText("not there"));
+      .isThrownBy(() -> select.selectByVisibleText("not there"));
+  }
+
+  @Test
+  public void shouldThrowExceptionOnSelectByVisibleTextIfOptionDisabled() {
+    WebElement selectElement = driver.findElement(By.name("single_disabled"));
+    Select select = new Select(selectElement);
+
+    assertThatExceptionOfType(UnsupportedOperationException.class)
+      .isThrownBy(() -> select.selectByVisibleText("Disabled"));
   }
 
   @Test
@@ -170,6 +185,15 @@ public void shouldThrowExceptionOnSelectByIndexIfOptionDoesNotExist() {
         .isThrownBy(() -> select.selectByIndex(10));
   }
 
+  @Test
+  public void shouldThrowExceptionOnSelectByIndexIfOptionDisabled() {
+    WebElement selectElement = driver.findElement(By.name("single_disabled"));
+    Select select = new Select(selectElement);
+
+    assertThatExceptionOfType(UnsupportedOperationException.class)
+      .isThrownBy(() -> select.selectByIndex(1));
+  }
+
   @Test
   public void shouldAllowOptionsToBeSelectedByReturnedValue() {
     WebElement selectElement = driver.findElement(By.name("select_empty_multiple"));
@@ -188,6 +212,15 @@ public void shouldThrowExceptionOnSelectByReturnedValueIfOptionDoesNotExist() {
         .isThrownBy(() -> select.selectByValue("not there"));
   }
 
+  @Test
+  public void shouldThrowExceptionOnSelectByReturnedValueIfOptionDisabled() {
+    WebElement selectElement = driver.findElement(By.name("single_disabled"));
+    Select select = new Select(selectElement);
+
+    assertThatExceptionOfType(UnsupportedOperationException.class)
+      .isThrownBy(() -> select.selectByValue("disabled"));
+  }
+
   @Test
   public void shouldAllowUserToDeselectAllWhenSelectSupportsMultipleSelections() {
     WebElement selectElement = driver.findElement(By.name("multi"));
diff --git a/java/test/org/openqa/selenium/support/ui/SelectTest.java b/java/test/org/openqa/selenium/support/ui/SelectTest.java
@@ -172,23 +172,6 @@ public void shouldNotAllowDisabledOptionsToBeSelected() {
     verify(firstOption, never()).click();
   }
 
-  @Test
-  public void shouldNotAllowOptionsToBeSelectedInDisabledSelect() {
-    final WebElement firstOption = mockOption("first", false);
-
-    final WebElement element = mockSelectWebElement("multiple");
-    when(element.isEnabled()).thenReturn(false);
-    when(element.findElements(By.xpath(".//option[normalize-space(.) = \"fish\"]")))
-        .thenReturn(Collections.singletonList(firstOption));
-
-    Select select = new Select(element);
-    assertThatThrownBy(() -> select.selectByVisibleText("fish"))
-      .isInstanceOf(UnsupportedOperationException.class)
-      .hasMessage("You may not select an option in disabled select");
-
-    verify(firstOption, never()).click();
-  }
-
   @Test
   public void shouldAllowOptionsToBeSelectedByIndex() {
     final WebElement firstOption = mockOption("first", true, 0);
