Fix disappearing ComboBox value (#19221)

Without the fix, when selecting a value from page n+1 and re-opening the
ComboBox popup, the content of the text box may disappear.

Change-Id: I5fb57c0e5a068645f4b11fb1c392e682dd04b06a

client/src/main/java/com/vaadin/client/ui/VFilterSelect.java

@@ -1871,6 +1871,18 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
             // currentPage = -1; // forget the page
         }
+
+        if (getSelectedCaption() != null && newKey.equals("")) {
+            // In scrollToPage(false) mode selecting null seems to be broken
+            // if current selection is not on first page. The above clause is so
+            // hard to interpret that new clause added here :-(
+            selectedOptionKey = newKey;
+            explicitSelectedCaption = null;
+            client.updateVariable(paintableId, "selected",
+                    new String[] { selectedOptionKey }, immediate);
+            afterUpdateClientVariables();
+        }
+
         suggestionPopup.hide();
     }
 
@@ -2326,6 +2338,8 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
      */
     boolean preventNextBlurEventInIE = false;
 
+    private String explicitSelectedCaption;
+
     /*
      * (non-Javadoc)
      * 
@@ -2366,7 +2380,11 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         focused = false;
         if (!readonly) {
             if (selectedOptionKey == null) {
-                setPromptingOn();
+                if (explicitSelectedCaption != null) {
+                    setPromptingOff(explicitSelectedCaption);
+                } else {
+                    setPromptingOn();
+                }
             } else if (currentSuggestion != null) {
                 setPromptingOff(currentSuggestion.caption);
             }
@@ -2575,4 +2593,29 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
                 || suggestionPopup.lazyPageScroller.isRunning();
     }
 
+    /**
+     * Sets the caption of selected item, if "scroll to page" is disabled.
+     * This method is meant for internal use and may change in future versions.
+     * 
+     * @since 7.7
+     * @param selectedCaption
+     *            the caption of selected item
+     */
+    public void setSelectedCaption(String selectedCaption) {
+        explicitSelectedCaption = selectedCaption;
+        if (selectedCaption != null) {
+            setPromptingOff(selectedCaption);
+        }
+    }
+
+    /**
+     * This method is meant for internal use and may change in future versions.
+     * 
+     * @since 7.7
+     * @return the caption of selected item, if "scroll to page" is disabled
+     */
+    public String getSelectedCaption() {
+        return explicitSelectedCaption;
+    }
+
 }

client/src/main/java/com/vaadin/client/ui/combobox/ComboBoxConnector.java

@@ -35,8 +35,8 @@ import com.vaadin.shared.ui.combobox.FilteringMode;
 import com.vaadin.ui.ComboBox;
 
 @Connect(ComboBox.class)
-public class ComboBoxConnector extends AbstractFieldConnector
-        implements Paintable, SimpleManagedLayout {
+public class ComboBoxConnector extends AbstractFieldConnector implements
+        Paintable, SimpleManagedLayout {
 
     // oldSuggestionTextMatchTheOldSelection is used to detect when it's safe to
     // update textbox text by a changed item caption.
@@ -65,16 +65,15 @@ public class ComboBoxConnector extends AbstractFieldConnector
         // work without additional UIDL messages
         boolean noTextInput = uidl
                 .hasAttribute(ComboBoxConstants.ATTR_NO_TEXT_INPUT)
-                && uidl.getBooleanAttribute(
-                        ComboBoxConstants.ATTR_NO_TEXT_INPUT);
+                && uidl.getBooleanAttribute(ComboBoxConstants.ATTR_NO_TEXT_INPUT);
         getWidget().setTextInputEnabled(!noTextInput);
 
         // not a FocusWidget -> needs own tabindex handling
         getWidget().tb.setTabIndex(getState().tabIndex);
 
         if (uidl.hasAttribute("filteringmode")) {
-            getWidget().filteringmode = FilteringMode
-                    .valueOf(uidl.getStringAttribute("filteringmode"));
+            getWidget().filteringmode = FilteringMode.valueOf(uidl
+                    .getStringAttribute("filteringmode"));
         }
 
         getWidget().immediate = getState().immediate;
@@ -105,6 +104,13 @@ public class ComboBoxConnector extends AbstractFieldConnector
             getWidget().suggestionPopupWidth = null;
         }
 
+        if (uidl.hasAttribute("suggestionPopupWidth")) {
+            getWidget().suggestionPopupWidth = uidl
+                    .getStringAttribute("suggestionPopupWidth");
+        } else {
+            getWidget().suggestionPopupWidth = null;
+        }
+
         getWidget().suggestionPopup.updateStyleNames(uidl, getState());
 
         getWidget().allowNewItem = uidl.hasAttribute("allownewitem");
@@ -181,12 +187,16 @@ public class ComboBoxConnector extends AbstractFieldConnector
             // started.
             if (selectedKeys.length > 0 && !selectedKeys[0].equals("")) {
                 performSelection(selectedKeys[0]);
+                // if selected key is available, assume caption is know based on
+                // it as well and clear selected caption
+                getWidget().setSelectedCaption(null);
+
             } else if (!getWidget().waitingForFilteringResponse
                     && uidl.hasAttribute("selectedCaption")) {
                 // scrolling to correct page is disabled, caption is passed as a
                 // special parameter
-                getWidget().tb
-                        .setText(uidl.getStringAttribute("selectedCaption"));
+                getWidget().setSelectedCaption(
+                        uidl.getStringAttribute("selectedCaption"));
             } else {
                 resetSelection();
             }
@@ -274,16 +284,16 @@ public class ComboBoxConnector extends AbstractFieldConnector
             if (!getWidget().waitingForFilteringResponse
                     || getWidget().popupOpenerClicked) {
                 if (!suggestionKey.equals(getWidget().selectedOptionKey)
-                        || suggestion.getReplacementString()
-                                .equals(getWidget().tb.getText())
+                        || suggestion.getReplacementString().equals(
+                                getWidget().tb.getText())
                         || oldSuggestionTextMatchTheOldSelection) {
                     // Update text field if we've got a new
                     // selection
                     // Also update if we've got the same text to
                     // retain old text selection behavior
                     // OR if selected item caption is changed.
-                    getWidget()
-                            .setPromptingOff(suggestion.getReplacementString());
+                    getWidget().setPromptingOff(
+                            suggestion.getReplacementString());
                     getWidget().selectedOptionKey = suggestionKey;
                 }
             }
@@ -296,8 +306,8 @@ public class ComboBoxConnector extends AbstractFieldConnector
 
     private boolean isWidgetsCurrentSelectionTextInTextBox() {
         return getWidget().currentSuggestion != null
-                && getWidget().currentSuggestion.getReplacementString()
-                        .equals(getWidget().tb.getText());
+                && getWidget().currentSuggestion.getReplacementString().equals(
+                        getWidget().tb.getText());
     }
 
     private void resetSelection() {
@@ -319,9 +329,18 @@ public class ComboBoxConnector extends AbstractFieldConnector
                 // just clear the input if the value has changed from something
                 // else to null
                 if (getWidget().selectedOptionKey != null
-                        || (getWidget().allowNewItem
-                                && !getWidget().tb.getValue().isEmpty())) {
+                        || (getWidget().allowNewItem && !getWidget().tb
+                                .getValue().isEmpty())) {
+
+                    boolean openedPopupWithNonScrollingMode = (getWidget().popupOpenerClicked
+                            && getWidget().getSelectedCaption() != null);
+                    if (!openedPopupWithNonScrollingMode) {
                     getWidget().tb.setValue("");
+                    } else {
+                        getWidget().tb
+                                .setValue(getWidget().getSelectedCaption());
+                        getWidget().tb.selectAll();
+                    }
                 }
             }
             getWidget().currentSuggestion = null; // #13217

uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxScrollingToPageDisabled.java

@@ -63,7 +63,7 @@ public class ComboBoxScrollingToPageDisabled extends
     protected String getDescription() {
         return "Test that selected value appears on the client "
                 + "side even though setScrollToSelectedItem(false) "
-                + "has been called. Textbox should containe 'Item 50'.";
+                + "has been called. Textbox should contain 'Item 50'.";
     }
 
     @Override

uitest/src/test/java/com/vaadin/tests/components/combobox/ComboBoxScrollingToPageDisabledTest.java

@@ -16,10 +16,10 @@
 package com.vaadin.tests.components.combobox;
 
 import org.junit.Test;
-import org.openqa.selenium.WebElement;
 
-import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.LabelElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.newelements.ComboBoxElement;
 
 /**
  * When pressed down key, while positioned on the last item - should show next
@@ -34,11 +34,20 @@ public class ComboBoxScrollingToPageDisabledTest extends MultiBrowserTest {
     }
 
     @Test
-    public void checkValueIsVidinlr() throws InterruptedException {
-        WebElement input = driver.findElement(By
-                .className("v-filterselect-input"));
-        String value = input.getAttribute("value");
-        org.junit.Assert.assertEquals("Item 50", value);
+    public void checkValueIsVisible() throws InterruptedException {
+        ComboBoxElement combo = $(ComboBoxElement.class).first();
+        org.junit.Assert.assertEquals("Item 50", combo.getText());
     }
 
+    @Test
+    public void checkLastValueIsVisible() throws InterruptedException {
+        ComboBoxElement combo = $(ComboBoxElement.class).first();
+        combo.selectByText("Item 99");
+        // this shouldn't clear the selection
+        combo.openPopup();
+        // close popup
+        $(LabelElement.class).first().click();
+
+        org.junit.Assert.assertEquals("Item 99", combo.getText());
+    }
 }