Set value selected by mouse when pressing Enter in ComboBox (#16981)

With this patch pressing Enter in ComboBox sets value selected by mouse.
Selection by keyboard is not given higher priority.

Change-Id: I2e5f610923a40f67be2c1504a004af0d431a67a3

Conflicts:
	client/src/com/vaadin/client/ui/VFilterSelect.java

3 files changed, 176 insertions, 53 deletions

diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java
index c0575b1ea5..6a3f0e9100 100644
--- a/client/src/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/com/vaadin/client/ui/VFilterSelect.java
@@ -454,7 +454,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
             // Set the text.
             setText(suggestion.getReplacementString());
 
-            menu.updateKeyboardSelectedItem();
         }
 
         /*
@@ -739,13 +738,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
     public class SuggestionMenu extends MenuBar implements SubPartAware,
             LoadHandler {
 
-        /**
-         * Tracks the item that is currently selected using the keyboard. This
-         * is need only because mouseover changes the selection and we do not
-         * want to use that selection when pressing enter to select the item.
-         */
-        private MenuItem keyboardSelectedItem;
-
         private VLazyExecutor delayedImageLoadExecutioner = new VLazyExecutor(
                 100, new ScheduledCommand() {
 
@@ -807,9 +799,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
             if (enableDebug) {
                 debug("VFS.SM: setSuggestions(" + suggestions + ")");
             }
-            // Reset keyboard selection when contents is updated to avoid
-            // reusing old, invalid data
-            setKeyboardSelectedItem(null);
 
             clearItems();
             final Iterator<FilterSelectSuggestion> it = suggestions.iterator();
@@ -971,14 +960,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
         }
 
-        private MenuItem getKeyboardSelectedItem() {
-            return keyboardSelectedItem;
-        }
-
-        public void setKeyboardSelectedItem(MenuItem menuItem) {
-            keyboardSelectedItem = menuItem;
-        }
-
         /**
          * @deprecated use {@link SuggestionPopup#selectFirstItem()} instead.
          */
@@ -1001,13 +982,6 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         }
 
         /*
-         * Sets the keyboard item as the current selected one.
-         */
-        void updateKeyboardSelectedItem() {
-            setKeyboardSelectedItem(getSelectedItem());
-        }
-
-        /*
          * Gets the height of one menu item.
          */
         int getItemOffsetHeight() {
@@ -1779,34 +1753,12 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
             event.stopPropagation();
             break;
         case KeyCodes.KEY_ENTER:
-            if (suggestionPopup.menu.getKeyboardSelectedItem() == null) {
-                /*
-                 * Nothing selected using up/down. Happens e.g. when entering a
-                 * text (causes popup to open) and then pressing enter.
-                 */
-                if (!allowNewItem) {
-                    /*
-                     * New items are not allowed: If there is only one
-                     * suggestion, select that. If there is more than one
-                     * suggestion Enter key should work as Escape key. Otherwise
-                     * do nothing.
-                     */
-                    if (currentSuggestions.size() == 1) {
-                        onSuggestionSelected(currentSuggestions.get(0));
-                    } else if (currentSuggestions.size() > 1) {
-                        reset();
-                    }
-                } else {
-                    // Handle addition of new items.
-                    suggestionPopup.menu.doSelectedItemAction();
-                }
+            if (!allowNewItem) {
+                onSuggestionSelected(currentSuggestions
+                        .get(suggestionPopup.menu.getSelectedIndex()));
             } else {
-                /*
-                 * Get the suggestion that was navigated to using up/down.
-                 */
-                currentSuggestion = ((FilterSelectSuggestion) suggestionPopup.menu
-                        .getKeyboardSelectedItem().getCommand());
-                onSuggestionSelected(currentSuggestion);
+                // Handle addition of new items.
+                suggestionPopup.menu.doSelectedItemAction();
             }
 
             event.stopPropagation();
diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnter.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnter.java
new file mode 100644
index 0000000000..5af4749349
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnter.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.combobox;
+
+import com.vaadin.data.Property;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.Label;
+
+public class ComboBoxMouseSelectEnter extends AbstractTestUI {
+    protected ComboBox comboBox;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        comboBox = new ComboBox();
+        final Label label = new Label();
+        label.setId("value");
+
+        comboBox.setTextInputAllowed(true);
+        comboBox.setNullSelectionAllowed(true);
+        comboBox.setNullSelectionItemId(null);
+
+        for (int i = 0; i < 10; i++) {
+            comboBox.addItem("a" + i);
+        }
+
+        comboBox.addValueChangeListener(new Property.ValueChangeListener() {
+            @Override
+            public void valueChange(Property.ValueChangeEvent event) {
+                Object value = event.getProperty().getValue();
+                if (value != null) {
+                    label.setValue(value.toString());
+                } else {
+                    label.setValue("null");
+                }
+            }
+        });
+
+        addComponents(comboBox);
+        addComponent(label);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Pressing Enter should set value highlighted from mouse position after using arrow keys";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 16981;
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnterTest.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnterTest.java
new file mode 100644
index 0000000000..d3ba37682d
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxMouseSelectEnterTest.java
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.combobox;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.parallel.BrowserUtil;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.newelements.ComboBoxElement;
+
+public class ComboBoxMouseSelectEnterTest extends MultiBrowserTest {
+
+    private ComboBoxElement comboBoxElement;
+
+    @Override
+    public void setup() throws Exception {
+
+        super.setup();
+        openTestURL();
+        waitForElementPresent(By.className("v-filterselect"));
+        comboBoxElement = $(ComboBoxElement.class).first();
+    }
+
+    @Test
+    public void enterSetsValueSelectedByMouseOver() {
+        comboBoxElement.openPopup();
+        comboBoxElement.sendKeys(Keys.DOWN, Keys.DOWN);
+        String selectedItemText = findElement(
+                By.className("gwt-MenuItem-selected")).getText();
+        assertThat("Item selected by arrows should be a1", selectedItemText,
+                is("a1"));
+        new Actions(driver).moveToElement(getWebElementForItem("a5")).build()
+                .perform();
+        comboBoxElement.sendKeys(getReturn());
+        assertThat("Item selected by mouse should be a5",
+                comboBoxElement.getText(), is("a5"));
+        checkLabelValue("a5");
+    }
+
+    private WebElement getWebElementForItem(String wantedText) {
+        WebElement wantedItem = null;
+        List<WebElement> items = findElements(By.className("gwt-MenuItem"));
+        for (WebElement item : items) {
+            if (item.getText().equals(wantedText)) {
+                wantedItem = item;
+                break;
+            }
+        }
+        return wantedItem;
+    }
+
+    private Keys getReturn() {
+        if (BrowserUtil.isPhantomJS(getDesiredCapabilities())) {
+            return Keys.ENTER;
+        } else {
+            return Keys.RETURN;
+        }
+    }
+
+    private void checkLabelValue(final String expectedValue) {
+
+        waitUntil(new ExpectedCondition<Boolean>() {
+            private String actualValue;
+
+            @Override
+            public Boolean apply(WebDriver input) {
+                actualValue = $(LabelElement.class).id("value").getText();
+                return actualValue.equals(expectedValue);
+            }
+
+            @Override
+            public String toString() {
+                // Timed out after 10 seconds waiting for ...
+                return String.format("Label value to match '%s' (was: '%s')",
+                        expectedValue, actualValue);
+            }
+        });
+    }
+
+}
\ No newline at end of file