Do not wait for icons to load before interacting with a combo box (#9717)

Icon load detection code does not handle error events so broken images
will cause tests to fail forever. In other cases also, it seems very
rare to need icons to be loaded before interacting with a combo box. If
there really are use cases for this, it would be better to have a method
to query if all icons are loaded so that special tests can use this method.

Fixes https://github.com/vaadin/testbench/issues/916

3 files changed, 27 insertions, 15 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 61e9b46e68..bb75385007 100644
--- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java
+++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java
@@ -1989,12 +1989,10 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
                     ClickEvent.getType());
             selectedItemIcon.addDomHandler(VComboBox.this,
                     MouseDownEvent.getType());
-            iconUpdating = true;
             selectedItemIcon.addDomHandler(new LoadHandler() {
                 @Override
                 public void onLoad(LoadEvent event) {
                     afterSelectedItemIconChange();
-                    iconUpdating = false;
                 }
             }, LoadEvent.getType());
             panel.insert(selectedItemIcon, 0);
@@ -2449,7 +2447,6 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
     boolean preventNextBlurEventInIE = false;
 
     private String explicitSelectedCaption;
-    private boolean iconUpdating = false;
 
     /*
      * (non-Javadoc)
@@ -2709,7 +2706,7 @@ public class VComboBox extends Composite implements Field, KeyDownHandler,
     @Override
     public boolean isWorkPending() {
         return dataReceivedHandler.isWaitingForFilteringResponse()
-                || suggestionPopup.lazyPageScroller.isRunning() || iconUpdating;
+                || suggestionPopup.lazyPageScroller.isRunning();
     }
 
     /**
diff --git a/uitest/src/main/java/com/vaadin/tests/elements/combobox/ComboBoxUI.java b/uitest/src/main/java/com/vaadin/tests/elements/combobox/ComboBoxUI.java
index d92130c863..08c1a51a7b 100644
--- a/uitest/src/main/java/com/vaadin/tests/elements/combobox/ComboBoxUI.java
+++ b/uitest/src/main/java/com/vaadin/tests/elements/combobox/ComboBoxUI.java
@@ -18,14 +18,13 @@ package com.vaadin.tests.elements.combobox;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.ThemeResource;
 import com.vaadin.server.VaadinRequest;
 import com.vaadin.tests.components.AbstractTestUI;
 import com.vaadin.ui.ComboBox;
 
-/**
- *
- */
-@SuppressWarnings("serial")
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class ComboBoxUI extends AbstractTestUI {
 
     public static final List<String> currencies = new ArrayList<String>();
@@ -37,12 +36,24 @@ public class ComboBoxUI extends AbstractTestUI {
 
     @Override
     protected void setup(VaadinRequest request) {
-        ComboBox comboBox = new ComboBox("NullAllowedComboBox", currencies);
+        ComboBox<String> comboBox = new ComboBox<>("NullAllowedComboBox",
+                currencies);
         addComponent(comboBox);
 
-        comboBox = new ComboBox("NullForbiddenComboBox", currencies);
+        comboBox = new ComboBox<>("NullForbiddenComboBox", currencies);
         comboBox.setEmptySelectionAllowed(false);
         addComponent(comboBox);
+
+        comboBox = new ComboBox<>("With icons", currencies);
+        comboBox.setId("with-icons");
+        comboBox.setItemIconGenerator(item -> {
+            if (item.equals("EUR")) {
+                return new ThemeResource("shared/img/spinner.gif");
+            } else {
+                return new ThemeResource("notfound.png");
+            }
+        });
+        addComponent(comboBox);
     }
 
     @Override
diff --git a/uitest/src/test/java/com/vaadin/tests/elements/combobox/ComboBoxUITest.java b/uitest/src/test/java/com/vaadin/tests/elements/combobox/ComboBoxUITest.java
index fb84ea3055..cecd748c9d 100644
--- a/uitest/src/test/java/com/vaadin/tests/elements/combobox/ComboBoxUITest.java
+++ b/uitest/src/test/java/com/vaadin/tests/elements/combobox/ComboBoxUITest.java
@@ -24,11 +24,6 @@ import org.openqa.selenium.WebElement;
 import com.vaadin.testbench.elements.ComboBoxElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
-/**
- * Validates that multiple calls to ComboBoxElement.selectByText(String) do not
- * append the input given each time to the previous one. The value in the
- * combobox's search field should be cleared before searching for a new one.
- */
 public class ComboBoxUITest extends MultiBrowserTest {
 
     @Before
@@ -54,6 +49,15 @@ public class ComboBoxUITest extends MultiBrowserTest {
         cb.selectByText("foobar");
     }
 
+    @Test
+    public void testSelectByTextWithIcons() {
+        ComboBoxElement cb = $(ComboBoxElement.class).id("with-icons");
+        cb.selectByText("GBP");
+        assertEquals("GBP", cb.getValue());
+        cb.selectByText("EUR");
+        assertEquals("EUR", cb.getValue());
+    }
+
     private void testMultipleSelectByTextOperationsIn(
             ComboBoxElement comboBox) {
         // Select all items from the menu