Revert "Update ComboBox internal state on new item added (#11094)" (#11331)

* Revert "Update ComboBox internal state on new item added (#11094)"

This reverts commit 56ce91c6160a252ddcd952bca6eb7037120ebf59.

* Add tests to verify the issue

7 files changed, 63 insertions, 39 deletions

diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
index a8ab05770e..f80927ec1d 100644
--- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java
+++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
@@ -16,6 +16,16 @@
 
 package com.vaadin.client.ui;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+import java.util.UUID;
+import java.util.logging.Logger;
+
 import com.google.gwt.animation.client.AnimationScheduler;
 import com.google.gwt.aria.client.Roles;
 import com.google.gwt.core.client.JavaScriptObject;
@@ -75,15 +85,6 @@ import com.vaadin.shared.AbstractComponentState;
 import com.vaadin.shared.ui.ComponentStateUtil;
 import com.vaadin.shared.util.SharedUtil;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Set;
-import java.util.logging.Logger;
-
 /**
  * Client side implementation of the ComboBox component.
  *
@@ -1635,11 +1636,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             performSelection(selectedKey, oldSuggestionTextMatchTheOldSelection,
                     !isWaitingForFilteringResponse() || popupOpenerClicked);
 
-            // currentSuggestion should be set to match the value of the
-            // ComboBox, especially when a new item is added.
-            resetCurrentSuggestionIfNecessary(selectedKey, selectedCaption,
-                    selectedIconUri);
-
             cancelPendingPostFiltering();
 
             setSelectedCaption(selectedCaption);
@@ -1647,16 +1643,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             setSelectedItemIcon(selectedIconUri);
         }
 
-        private void resetCurrentSuggestionIfNecessary(String selectedKey,
-                String selectedCaption, String selectedIconUri) {
-            if (currentSuggestion == null
-                    && (selectedKey != null || selectedCaption != null))
-                currentSuggestion = new ComboBoxSuggestion(selectedKey,
-                        selectedCaption, "", selectedIconUri);
-            else if (selectedKey == null && selectedCaption == null)
-                currentSuggestion = null;
-        }
-
     }
 
     // TODO decide whether this should change - affects themes and v7
@@ -2124,7 +2110,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
             currentSuggestion = null; // #13217
             selectedOptionKey = null;
             setText(getEmptySelectionCaption());
-            return;
         }
         // some item selected
         for (ComboBoxSuggestion suggestion : currentSuggestions) {
diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index 2d613a90a8..0a5a36c856 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -186,23 +186,20 @@ public class ComboBox<T> extends AbstractSingleSelect<T>
         @Override
         public void createNewItem(String itemValue) {
             // New option entered
-            boolean clientSideHandling = false;
+            boolean added = false;
             if (itemValue != null && !itemValue.isEmpty()) {
                 if (getNewItemProvider() != null) {
-                    getNewItemProvider().apply(itemValue).ifPresent(value -> {
-                        // Update state for the newly selected value
-                        setSelectedItem(value, true);
-                        getDataCommunicator().reset();
-                    });
+                    Optional<T> item = getNewItemProvider().apply(itemValue);
+                    added = item.isPresent();
                 } else if (getNewItemHandler() != null) {
                     getNewItemHandler().accept(itemValue);
                     // Up to the user to tell if no item was added.
-                    clientSideHandling = true;
+                    added = true;
                 }
             }
 
-            if (!clientSideHandling) {
-                // New item was maybe added with NewItemHandler
+            if (!added) {
+                // New item was not handled.
                 getRpcProxy(ComboBoxClientRpc.class).newItemNotAdded(itemValue);
             }
         }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerReset.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerReset.java
new file mode 100644
index 0000000000..f50cf8a92b
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerReset.java
@@ -0,0 +1,5 @@
+package com.vaadin.tests.components.combobox;
+
+public class ComboBoxAddingSameItemTwoTimesWithItemHandlerReset
+        extends ComboBoxSelectingNewItemValueChange {
+}
diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderReset.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderReset.java
new file mode 100644
index 0000000000..d43214eed3
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderReset.java
@@ -0,0 +1,5 @@
+package com.vaadin.tests.components.combobox;
+
+public class ComboBoxAddingSameItemTwoTimesWithItemProviderReset
+        extends ComboBoxNewItemProvider {
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerResetTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerResetTest.java
new file mode 100644
index 0000000000..dbaa3a648f
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemHandlerResetTest.java
@@ -0,0 +1,24 @@
+package com.vaadin.tests.components.combobox;
+
+public class ComboBoxAddingSameItemTwoTimesWithItemHandlerResetTest
+        extends ComboBoxSelectingNewItemValueChangeTest {
+
+    @Override
+    public void itemHandling(
+            ComboBoxSelectingNewItemValueChangeTest.SelectionType selectionType,
+            String[] inputs) {
+        assertThatSelectedValueIs("");
+
+        // add new item for the first time
+        typeInputAndSelect(inputs[0], selectionType);
+        assertThatSelectedValueIs(inputs[0]);
+        assertValueChange(1);
+
+        reset();
+
+        // add the same item for the 2nd time
+        typeInputAndSelect(inputs[0], selectionType);
+        assertThatSelectedValueIs(inputs[0]);
+        assertValueChange(1);
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderResetTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderResetTest.java
new file mode 100644
index 0000000000..d2ed70d211
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxAddingSameItemTwoTimesWithItemProviderResetTest.java
@@ -0,0 +1,6 @@
+package com.vaadin.tests.components.combobox;
+
+public class ComboBoxAddingSameItemTwoTimesWithItemProviderResetTest
+        extends ComboBoxAddingSameItemTwoTimesWithItemHandlerResetTest {
+    // same tests using NewItemProvider instead of NewItemHandler
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChangeTest.java
index 0707ebd020..1e2869df61 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChangeTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxSelectingNewItemValueChangeTest.java
@@ -1,6 +1,7 @@
 package com.vaadin.tests.components.combobox;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
 import org.openqa.selenium.Keys;
@@ -18,7 +19,7 @@ import com.vaadin.tests.tb3.MultiBrowserTest;
 
 public class ComboBoxSelectingNewItemValueChangeTest extends MultiBrowserTest {
 
-    private enum SelectionType {
+    protected enum SelectionType {
         ENTER, TAB, CLICK_OUT;
     }
 
@@ -134,7 +135,8 @@ public class ComboBoxSelectingNewItemValueChangeTest extends MultiBrowserTest {
         assertItemCount(2602);
     }
 
-    private void typeInputAndSelect(String input, SelectionType selectionType) {
+    protected void typeInputAndSelect(String input,
+            SelectionType selectionType) {
         comboBoxElement.clear();
         sendKeysToInput(input);
         switch (selectionType) {
@@ -166,7 +168,7 @@ public class ComboBoxSelectingNewItemValueChangeTest extends MultiBrowserTest {
         }
     }
 
-    private void assertThatSelectedValueIs(final String value) {
+    protected void assertThatSelectedValueIs(final String value) {
         waitUntil(new ExpectedCondition<Boolean>() {
             private String actualComboBoxValue;
             private String actualLabelValue;
@@ -189,7 +191,7 @@ public class ComboBoxSelectingNewItemValueChangeTest extends MultiBrowserTest {
         });
     }
 
-    private void assertValueChange(int count) {
+    protected void assertValueChange(int count) {
         assertEquals(String.format(
                 "Value change count: %s Selection change count: %s user originated: true",
                 count, count), changeLabelElement.getText());
@@ -226,7 +228,7 @@ public class ComboBoxSelectingNewItemValueChangeTest extends MultiBrowserTest {
         }
     }
 
-    private void reset() {
+    protected void reset() {
         $(ButtonElement.class).id("reset").click();
     }