Fix animation end listeners so they are always removed (#17903)

Fixes ComboBox suggestion popup so that it will not automatically
close when clicking the popup button, if the user happened to
double click on the button earlier.

Ported from 7.7

Change-Id: I6cd8c7744ca4c52a7bd52ab12c23fb55522f0611

diff --git a/client/src/main/java/com/vaadin/client/AnimationUtil.java b/client/src/main/java/com/vaadin/client/AnimationUtil.java
index 652cfd9432745bcef0000beac11e93f7b991410d..bbefcffbdd652df5111642d650b8f4d4e8e4e8cc 100644 (file)
--- a/client/src/main/java/com/vaadin/client/AnimationUtil.java
+++ b/client/src/main/java/com/vaadin/client/AnimationUtil.java
@@ -19,6 +19,7 @@ import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
 import com.google.gwt.dom.client.Style;
+import com.vaadin.client.AnimationUtil.AnimationEndListener;
 
 /**
  * Utility methods for working with CSS transitions and animations.
@@ -65,6 +66,7 @@ public class AnimationUtil {
       var callbackFunc = $entry(function(e) {
         listener.@com.vaadin.client.AnimationUtil.AnimationEndListener::onAnimationEnd(Lcom/google/gwt/dom/client/NativeEvent;)(e);
       });
+      callbackFunc.listener = listener;
 
       elem.addEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, callbackFunc, false);
 
@@ -84,6 +86,31 @@ public class AnimationUtil {
       elem.removeEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, listener, false);
     }-*/;
 
+    /**
+     * Removes the given animation listener.
+     *
+     * @param element
+     *            the element which has the listener
+     * @param animationEndListener
+     *            the listener to remove
+     * @return <code>true</code> if the listener was removed, <code>false</code>
+     *         if the listener was not registered to the given element
+     */
+    public static native boolean removeAnimationEndListener(Element elem,
+            AnimationEndListener animationEndListener)
+    /*-{
+      if(elem._vaadin_animationend_callbacks) {
+        var callbacks = elem._vaadin_animationend_callbacks;
+        for(var i=0; i < callbacks.length; i++) {
+          if (callbacks[i].listener == animationEndListener) {
+              elem.removeEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, callbacks[i], false);
+              return true;
+          }
+        }
+        return false;
+      }
+    }-*/;
+
     /** For internal use only. May be removed or replaced in the future. */
     public static native void removeAllAnimationEndListeners(Element elem)
     /*-{

diff --git a/client/src/main/java/com/vaadin/client/widgets/Overlay.java b/client/src/main/java/com/vaadin/client/widgets/Overlay.java
index 35a5b46576906bd1295f7b0b843317417b7ec6d3..0b1d317a0560d2ea3a748b5e1e25d24072b890fd 100644 (file)
--- a/client/src/main/java/com/vaadin/client/widgets/Overlay.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Overlay.java
@@ -21,7 +21,6 @@ import java.util.List;
 
 import com.google.gwt.animation.client.Animation;
 import com.google.gwt.core.client.GWT;
-import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.IFrameElement;
@@ -396,8 +395,6 @@ public class Overlay extends PopupPanel {
         current = null;
     }
 
-    private JavaScriptObject animateInListener;
-
     private boolean fitInWindow = false;
 
     private boolean maybeShowWithAnimation() {
@@ -422,16 +419,18 @@ public class Overlay extends PopupPanel {
             if (animationName.contains(ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
                 // Disable GWT PopupPanel animation if used
                 setAnimationEnabled(false);
-                animateInListener = AnimationUtil.addAnimationEndListener(
-                        getElement(), new AnimationEndListener() {
+                AnimationUtil.addAnimationEndListener(getElement(),
+                        new AnimationEndListener() {
                             @Override
                             public void onAnimationEnd(NativeEvent event) {
                                 String animationName = AnimationUtil
                                         .getAnimationName(event);
                                 if (animationName.contains(
                                         ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
-                                    AnimationUtil.removeAnimationEndListener(
-                                            getElement(), animateInListener);
+                                    boolean removed = AnimationUtil
+                                            .removeAnimationEndListener(
+                                                    getElement(), this);
+                                    assert removed : "Animation end listener was not removed";
                                     removeStyleDependentName(
                                             ADDITIONAL_CLASSNAME_ANIMATE_IN);
                                 }
@@ -720,6 +719,10 @@ public class Overlay extends PopupPanel {
                         public void onAnimationEnd(NativeEvent event) {
                             if (AnimationUtil.getAnimationName(event).contains(
                                     ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
+                                boolean removed = AnimationUtil
+                                        .removeAnimationEndListener(
+                                                getElement(), this);
+                                assert removed : "Animation end listener was not removed";
                                 reallyHide(autoClosed);
                             }
                         }
@@ -746,11 +749,12 @@ public class Overlay extends PopupPanel {
                                         .getAnimationName(event);
                                 if (animationName.contains(
                                         ADDITIONAL_CLASSNAME_ANIMATE_OUT)) {
-                                    AnimationUtil
-                                            .removeAllAnimationEndListeners(
-                                                    getElement());
-                                    // Remove both animation styles just in
-                                    // case
+                                    boolean removed = AnimationUtil
+                                            .removeAnimationEndListener(
+                                                    getElement(), this);
+                                    assert removed : "Animation end listener was not removed";
+
+                                    // Remove both animation styles just in case
                                     removeStyleDependentName(
                                             ADDITIONAL_CLASSNAME_ANIMATE_IN);
                                     removeStyleDependentName(

diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java
new file mode 100644 (file)
index 0000000..7b32b5e
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java
@@ -0,0 +1,48 @@
+/*
+ * 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.annotations.Theme;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.v7.ui.ComboBox;
+
+@Theme("valo")
+public class ComboBoxValoDoubleClick extends AbstractTestUI {
+
+    // Quite impossible to autotest reliably as there must be a click to open
+    // the popup and another click during the opening animation to reproduce the
+    // bug. Manually a double click is just about the right timing.
+    @Override
+    protected void setup(VaadinRequest request) {
+        ComboBox cb = new ComboBox("Double-click Me");
+        for (int i = 0; i < 100; i++) {
+            cb.addItem("Item-" + i);
+        }
+        addComponent(cb);
+    }
+
+    @Override
+    public String getTestDescription() {
+        return "ComboBox should remain usable even after double-clicking (affects only Valo theme with $v-overlay-animate-in).";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 17903;
+    }
+
+}