Bug 994117, delay popupshown event until after the opening transition has finished, if one exists, which also fixes the issue causing bug 989991, so enable the animation on the main panel, r=neil

browser/components/customizableui/content/panelUI.inc.xul

@@ -7,7 +7,6 @@
        type="arrow"
        hidden="true"
        flip="slide"
-       animate="false"
        position="bottomcenter topright"
        noautofocus="true">
   <panelmultiview id="PanelUI-multiView" mainViewId="PanelUI-mainView">

layout/xul/nsMenuPopupFrame.cpp

@@ -44,6 +44,7 @@
 #include "nsIScreenManager.h"
 #include "nsIServiceManager.h"
 #include "nsThemeConstants.h"
+#include "nsTransitionManager.h"
 #include "nsDisplayList.h"
 #include "mozilla/EventDispatcher.h"
 #include "mozilla/EventStateManager.h"
@@ -330,26 +331,37 @@ nsMenuPopupFrame::GetShadowStyle()
   return NS_STYLE_WINDOW_SHADOW_DEFAULT;
 }
 
-// this class is used for dispatching popupshown events asynchronously.
-class nsXULPopupShownEvent : public nsRunnable
+NS_IMETHODIMP nsXULPopupShownEvent::Run()
 {
-public:
-  nsXULPopupShownEvent(nsIContent *aPopup, nsPresContext* aPresContext)
-    : mPopup(aPopup), mPresContext(aPresContext)
-  {
+  WidgetMouseEvent event(true, NS_XUL_POPUP_SHOWN, nullptr,
+                         WidgetMouseEvent::eReal);
+  return EventDispatcher::Dispatch(mPopup, mPresContext, &event);                 
+}
+
+NS_IMETHODIMP nsXULPopupShownEvent::HandleEvent(nsIDOMEvent* aEvent)
+{
+  nsMenuPopupFrame* popup = do_QueryFrame(mPopup->GetPrimaryFrame());
+  if (popup) {
+    // ResetPopupShownDispatcher will delete the reference to this, so keep
+    // another one until Run is finished.
+    nsRefPtr<nsXULPopupShownEvent> event = this;
+    // Only call Run if it the dispatcher was assigned. This avoids calling the
+    // Run method if the transitionend event fires multiple times.
+    if (popup->ClearPopupShownDispatcher()) {
+      return Run();
+    }
   }
 
-  NS_IMETHOD Run() MOZ_OVERRIDE
-  {
-    WidgetMouseEvent event(true, NS_XUL_POPUP_SHOWN, nullptr,
-                           WidgetMouseEvent::eReal);
-    return EventDispatcher::Dispatch(mPopup, mPresContext, &event);                 
-  }
+  CancelListener();
+  return NS_OK;
+}
 
-private:
-  nsCOMPtr<nsIContent> mPopup;
-  nsRefPtr<nsPresContext> mPresContext;
-};
+void nsXULPopupShownEvent::CancelListener()
+{
+  mPopup->RemoveSystemEventListener(NS_LITERAL_STRING("transitionend"), this, false);
+}
+
+NS_IMPL_ISUPPORTS_INHERITED(nsXULPopupShownEvent, nsRunnable, nsIDOMEventListener);
 
 void
 nsMenuPopupFrame::SetInitialChildList(ChildListID  aListID,
@@ -482,6 +494,21 @@ nsMenuPopupFrame::LayoutPopup(nsBoxLayoutState& aState, nsIFrame* aParentMenu,
   // finally, if the popup just opened, send a popupshown event
   if (mIsOpenChanged) {
     mIsOpenChanged = false;
+
+#ifndef MOZ_WIDGET_GTK
+    // If the animate attribute is set to open, check for a transition and wait
+    // for it to finish before firing the popupshown event.
+    if (mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::animate,
+                              nsGkAtoms::open, eCaseMatters) &&
+        nsLayoutUtils::HasCurrentAnimations(mContent, nsGkAtoms::transitionsProperty, pc)) {
+      mPopupShownDispatcher = new nsXULPopupShownEvent(mContent, pc);
+      mContent->AddSystemEventListener(NS_LITERAL_STRING("transitionend"),
+                                       mPopupShownDispatcher, false, false);
+      return;
+    }
+#endif
+
+    // If there are no transitions, fire the popupshown event right away.
     nsCOMPtr<nsIRunnable> event = new nsXULPopupShownEvent(GetContent(), pc);
     NS_DispatchToCurrentThread(event);
   }
@@ -779,6 +806,8 @@ nsMenuPopupFrame::HidePopup(bool aDeselectMenu, nsPopupState aNewState)
   NS_ASSERTION(aNewState == ePopupClosed || aNewState == ePopupInvisible,
                "popup being set to unexpected state");
 
+  ClearPopupShownDispatcher();
+
   // don't hide the popup when it isn't open
   if (mPopupState == ePopupClosed || mPopupState == ePopupShowing)
     return;
@@ -1887,6 +1916,8 @@ nsMenuPopupFrame::DestroyFrom(nsIFrame* aDestructRoot)
       new nsUnsetAttrRunnable(menu->GetContent(), nsGkAtoms::open));
   }
 
+  ClearPopupShownDispatcher();
+
   nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
   if (pm)
     pm->PopupDestroyed(this);

layout/xul/nsMenuPopupFrame.h

@@ -118,6 +118,28 @@ class nsViewManager;
 class nsView;
 class nsMenuPopupFrame;
 
+// this class is used for dispatching popupshown events asynchronously.
+class nsXULPopupShownEvent : public nsRunnable, public nsIDOMEventListener
+{
+public:
+  nsXULPopupShownEvent(nsIContent *aPopup, nsPresContext* aPresContext)
+    : mPopup(aPopup), mPresContext(aPresContext)
+  {
+  }
+
+  virtual ~nsXULPopupShownEvent() { }
+
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_NSIRUNNABLE
+  NS_DECL_NSIDOMEVENTLISTENER
+
+  void CancelListener();
+
+private:
+  nsCOMPtr<nsIContent> mPopup;
+  nsRefPtr<nsPresContext> mPresContext;
+};
+
 class nsMenuPopupFrame : public nsBoxFrame, public nsMenuParent
 {
 public:
@@ -345,6 +367,20 @@ public:
 
   // Return the offset applied to the alignment of the popup
   nscoord GetAlignmentOffset() const { return mAlignmentOffset; }
+
+  // Clear the mPopupShownDispatcher, remove the listener and return true if
+  // mPopupShownDispatcher was non-null.
+  bool ClearPopupShownDispatcher()
+  {
+    if (mPopupShownDispatcher) {
+      mPopupShownDispatcher->CancelListener();
+      mPopupShownDispatcher = nullptr;
+      return true;
+    }
+
+    return false;
+  }
+
 protected:
 
   // returns the popup's level.
@@ -434,6 +470,8 @@ protected:
 
   nsMenuFrame* mCurrentMenu; // The current menu that is active.
 
+  nsRefPtr<nsXULPopupShownEvent> mPopupShownDispatcher;
+
   // A popup's preferred size may be different than its actual size stored in
   // mRect in the case where the popup was resized because it was too large
   // for the screen. The preferred size mPrefSize holds the full size the popup

toolkit/content/tests/chrome/test_arrowpanel.xul

@@ -34,6 +34,7 @@
 </panel>
 
 <panel id="animatepanel" type="arrow"
+       onpopupshown="animatedPopupShown = true;"
        onpopuphidden="animatedPopupHidden = true; runNextTest.next();">
   <label value="Animate Closed" height="40"/>
 </panel>
@@ -46,6 +47,7 @@ SimpleTest.waitForExplicitFinish();
 var expectedAnchor = null;
 var expectedSide = "", expectedAnchorEdge = "", expectedPack = "", expectedAlignment = "";
 var zoomFactor = 1;
+var animatedPopupShown = false;
 var animatedPopupHidden = false;
 var runNextTest;
 
@@ -184,8 +186,12 @@ function nextTest()
       transitions++;
       // Two properties transition so continue on the second one finishing.
       if (!(transitions % 2)) {
+        ok(animatedPopupShown, "popupshown now fired")
         SimpleTest.executeSoon(() => runNextTest.next());
       }
+      else {
+        ok(!animatedPopupShown, "popupshown not fired yet")
+      }
     }
 
     // Check that the transition occurs for an arrow panel with animate="true"
@@ -193,6 +199,7 @@ function nextTest()
     $("animatepanel").openPopup($("topleft"), "after_start", 0, 0, false, false, null, "start");
     yield;
     window.removeEventListener("transitionend", transitionEnded, false);
+
     synthesizeKey("VK_ESCAPE", { });
     ok(!animatedPopupHidden, "animated popup not hidden yet");
     yield;