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

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 61e9b46e686dfa5c1dd067f2794031ba24e06f14..bb75385007a508cb704f24d41a7f1067c693084a 100644 (file)
--- 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 d92130c8632c71ddbe9fdef1e5900835bc599e1c..08c1a51a7b9b39cdf01a31af9c6381da1bbc7e39 100644 (file)
--- 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 fb84ea3055de767c46c1fdecabb715f062b63427..cecd748c9d5c2eae21079af3be942ab167673970 100644 (file)
--- 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