Fixes exception when enter is pressed and there is nothing to select …

…(#19149) Change-Id: I8ff11e98bb4ec999d369ff5bcde0cb95290ed037
vaadin · Jan 7, 2016 · af15d54 · af15d54
1 parent 86969a9
commit af15d54
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 4 deletions.
diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java
@@ -1861,8 +1861,16 @@ private void popupKeyDown(KeyDownEvent event) {
         case KeyCodes.KEY_ENTER:
 
             if (!allowNewItem) {
-                onSuggestionSelected(currentSuggestions
-                        .get(suggestionPopup.menu.getSelectedIndex()));
+                int selected = suggestionPopup.menu.getSelectedIndex();
+                if (selected != -1) {
+                    onSuggestionSelected(currentSuggestions.get(selected));
+                } else {
+                    // The way VFilterSelect is done, it handles enter and tab
+                    // in exactly the same way so we close the popup in both
+                    // cases even though we could leave it open when pressing
+                    // enter
+                    suggestionPopup.hide();
+                }
             } else {
                 // Handle addition of new items.
                 suggestionPopup.menu.doSelectedItemAction();

diff --git a/...st/src/com/vaadin/tests/components/combobox/ComboBoxEmptyItemsKeyboardNavigationTest.java b/...st/src/com/vaadin/tests/components/combobox/ComboBoxEmptyItemsKeyboardNavigationTest.java
@@ -10,10 +10,11 @@
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.By;
-import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.SingleBrowserTestPhantomJS2;
 import com.vaadin.tests.tb3.newelements.ComboBoxElement;
 
-public class ComboBoxEmptyItemsKeyboardNavigationTest extends MultiBrowserTest {
+public class ComboBoxEmptyItemsKeyboardNavigationTest extends
+        SingleBrowserTestPhantomJS2 {
 
     @Test
     public void navigatingUpOnAnEmptyMenuDoesntThrowErrors() {
@@ -27,4 +28,43 @@ public void navigatingUpOnAnEmptyMenuDoesntThrowErrors() {
 
         assertThat(errors, empty());
     }
+
+    @Test
+    public void selectingUsingEnterInAnEmptyMenu() {
+        setDebug(true);
+        openTestURL();
+
+        ComboBoxElement combobox = $(ComboBoxElement.class).first();
+        combobox.sendKeys("a", Keys.ENTER);
+
+        List<WebElement> errors = findElements(By.className("SEVERE"));
+
+        assertThat(errors, empty());
+
+        assertPopupClosed(combobox);
+    }
+
+    @Test
+    public void selectingUsingTabInAnEmptyMenu() {
+        setDebug(true);
+        openTestURL();
+
+        ComboBoxElement combobox = $(ComboBoxElement.class).first();
+        // The joy of testing, one tab should be enough but is not (it is
+        // locally), two tabs does the trick for PhantomJS on the cluster...
+        combobox.sendKeys("abc", Keys.TAB, Keys.TAB);
+
+        List<WebElement> errors = findElements(By.className("SEVERE"));
+
+        assertThat(errors, empty());
+        assertPopupClosed(combobox);
+    }
+
+    private void assertPopupClosed(ComboBoxElement combobox) {
+        org.openqa.selenium.By bySuggestionPopup = By.vaadin("#popup");
+
+        assertThat("ComboBox popup should not be open",
+                combobox.findElements(bySuggestionPopup).isEmpty());
+
+    }
 }