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.

Change-Id: I60936f82133ca89c2aef0dccff68092a11deff1c

diff --git a/client/src/main/java/com/vaadin/client/AnimationUtil.java b/client/src/main/java/com/vaadin/client/AnimationUtil.java
index 063a0a163e73e0d653f6ff552dbaa15eefa85560..780d2039e1bd4d088c8102af90973ddc30da84ab 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)
     /*-{
@@ -149,7 +176,7 @@ public class AnimationUtil {
           'MozAnimation': 'animationend',
           'WebkitAnimation': 'webkitAnimationEnd'
         }
-    
+
         for(var a in anims){
             if( el.style[a] !== undefined ){
                 return anims[a];
@@ -168,7 +195,7 @@ public class AnimationUtil {
           'mozAnimation',
           'webkitAnimation'
         ]
-    
+
         for(var i=0; i < anims.length; i++) {
             if( el.style[anims[i]] !== undefined ){
                 return anims[i];

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 ee20e1a9f1e87fc833712b0c233cb12cb456f64f..b10b1a1f3d1bf2120c43793a2c4327d3befe957e 100644 (file)
--- a/client/src/main/java/com/vaadin/client/widgets/Overlay.java
+++ b/client/src/main/java/com/vaadin/client/widgets/Overlay.java
@@ -545,8 +545,6 @@ public class Overlay extends PopupPanel implements CloseHandler<PopupPanel> {
         current = null;
     }
 
-    private JavaScriptObject animateInListener;
-
     private boolean fitInWindow = false;
 
     private boolean maybeShowWithAnimation() {
@@ -577,7 +575,7 @@ public class Overlay extends PopupPanel implements CloseHandler<PopupPanel> {
             if (animationName.contains(ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
                 // Disable GWT PopupPanel animation if used
                 setAnimationEnabled(false);
-                animateInListener = AnimationUtil.addAnimationEndListener(
+                AnimationUtil.addAnimationEndListener(
                         getElement(), new AnimationEndListener() {
                             @Override
                             public void onAnimationEnd(NativeEvent event) {
@@ -585,8 +583,8 @@ public class Overlay extends PopupPanel implements CloseHandler<PopupPanel> {
                                         .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);
                                     if (isShadowEnabled()) {
                                         shadow.removeClassName(CLASSNAME_SHADOW
@@ -1029,6 +1027,8 @@ public class Overlay extends PopupPanel implements CloseHandler<PopupPanel> {
                                         .getAnimationName(event)
                                         .contains(
                                                 ADDITIONAL_CLASSNAME_ANIMATE_IN)) {
+                                    boolean removed = AnimationUtil.removeAnimationEndListener(getElement(), this);
+                                    assert removed: "Animation end listener was not removed";
                                     reallyHide(autoClosed);
                                 }
                             }
@@ -1060,8 +1060,8 @@ public class Overlay extends PopupPanel implements CloseHandler<PopupPanel> {
                                             .getAnimationName(event);
                                     if (animationName
                                             .contains(ADDITIONAL_CLASSNAME_ANIMATE_OUT)) {
-                                        AnimationUtil
-                                                .removeAllAnimationEndListeners(getElement());
+                                        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);

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..f8886e3
--- /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.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;
+    }
+
+}