Bug 1294853 part1 - hide should preceed its related show on a move, r=yzen
authorAlexander Surkov <surkov.alexander@gmail.com>
Thu, 29 Sep 2016 15:44:18 -0400
changeset 315894 810a4f6dff0407c622e89cea3a6bb9a532c01cd0
parent 315893 e5049535533e2af84066c0ad7e2e2259b123d8ee
child 315895 93a20b9fe4f9716bf5a76e89e65cdb15d79585b4
push id20634
push usercbook@mozilla.com
push dateFri, 30 Sep 2016 10:10:13 +0000
treeherderfx-team@afe79b010d13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1294853
milestone52.0a1
Bug 1294853 part1 - hide should preceed its related show on a move, r=yzen
accessible/base/AccEvent.h
accessible/base/EventTree.cpp
accessible/base/EventTree.h
accessible/base/NotificationController.cpp
accessible/base/NotificationController.h
accessible/base/nsEventShell.cpp
accessible/generic/DocAccessible.cpp
accessible/tests/mochitest/events/test_coalescence.html
accessible/tests/mochitest/treeupdate/test_ariaowns.html
--- a/accessible/base/AccEvent.h
+++ b/accessible/base/AccEvent.h
@@ -5,16 +5,17 @@
 
 #ifndef _AccEvent_H_
 #define _AccEvent_H_
 
 #include "nsIAccessibleEvent.h"
 
 #include "mozilla/a11y/Accessible.h"
 
+class nsEventShell;
 namespace mozilla {
 
 namespace dom {
 class Selection;
 }
 
 namespace a11y {
 
@@ -126,16 +127,17 @@ protected:
 
   bool mIsFromUserInput;
   uint32_t mEventType;
   EEventRule mEventRule;
   RefPtr<Accessible> mAccessible;
 
   friend class EventQueue;
   friend class EventTree;
+  friend class ::nsEventShell;
 };
 
 
 /**
  * Accessible state change event.
  */
 class AccStateChangeEvent: public AccEvent
 {
@@ -285,17 +287,20 @@ public:
   virtual unsigned int GetEventGroups() const override
   {
     return AccMutationEvent::GetEventGroups() | (1U << eShowEvent);
   }
 
   uint32_t InsertionIndex() const { return mInsertionIndex; }
 
 private:
+  nsTArray<RefPtr<AccHideEvent>> mPrecedingEvents;
   uint32_t mInsertionIndex;
+
+  friend class EventTree;
 };
 
 
 /**
  * Class for reorder accessible event. Takes care about
  */
 class AccReorderEvent : public AccEvent
 {
--- a/accessible/base/EventTree.cpp
+++ b/accessible/base/EventTree.cpp
@@ -149,16 +149,34 @@ TreeMutation::PrefixLog(void* aData, Acc
 }
 #endif
 
 
 ////////////////////////////////////////////////////////////////////////////////
 // EventTree
 
 void
+EventTree::Shown(Accessible* aChild)
+{
+  RefPtr<AccShowEvent> ev = new AccShowEvent(aChild);
+  Controller(aChild)->WithdrawPrecedingEvents(&ev->mPrecedingEvents);
+  Mutated(ev);
+}
+
+void
+EventTree::Hidden(Accessible* aChild, bool aNeedsShutdown)
+{
+  RefPtr<AccHideEvent> ev = new AccHideEvent(aChild, aNeedsShutdown);
+  if (!aNeedsShutdown) {
+    Controller(aChild)->StorePrecedingEvent(ev);
+  }
+  Mutated(ev);
+}
+
+void
 EventTree::Process(const RefPtr<DocAccessible>& aDeathGrip)
 {
   while (mFirst) {
     // Skip a node and its subtree if its container is not in the document.
     if (mFirst->mContainer->IsInDocument()) {
       mFirst->Process(aDeathGrip);
       if (aDeathGrip->IsDefunct()) {
         return;
@@ -172,20 +190,29 @@ EventTree::Process(const RefPtr<DocAcces
   MOZ_ASSERT(!mContainer || !mContainer->IsDefunct(),
              "Processing events for defunct container");
   MOZ_ASSERT(!mFireReorder || mContainer, "No target for reorder event");
 
   // Fire mutation events.
   uint32_t eventsCount = mDependentEvents.Length();
   for (uint32_t jdx = 0; jdx < eventsCount; jdx++) {
     AccMutationEvent* mtEvent = mDependentEvents[jdx];
-    MOZ_ASSERT(mtEvent->mEventRule != AccEvent::eDoNotEmit,
-               "The event shouldn't be presented in the tree");
     MOZ_ASSERT(mtEvent->Document(), "No document for event target");
 
+    // Fire all hide events that has to be fired before this show event.
+    if (mtEvent->IsShow()) {
+      AccShowEvent* showEv = downcast_accEvent(mtEvent);
+      for (uint32_t i = 0; i < showEv->mPrecedingEvents.Length(); i++) {
+        nsEventShell::FireEvent(showEv->mPrecedingEvents[i]);
+        if (aDeathGrip->IsDefunct()) {
+          return;
+        }
+      }
+    }
+
     nsEventShell::FireEvent(mtEvent);
     if (aDeathGrip->IsDefunct()) {
       return;
     }
 
     if (mtEvent->mTextChangeEvent) {
       nsEventShell::FireEvent(mtEvent->mTextChangeEvent);
       if (aDeathGrip->IsDefunct()) {
@@ -410,23 +437,32 @@ EventTree::Log(uint32_t aLevel) const
   for (uint32_t i = 0; i < aLevel; i++) {
     printf("  ");
   }
   logging::AccessibleInfo("container", mContainer);
 
   for (uint32_t i = 0; i < mDependentEvents.Length(); i++) {
     AccMutationEvent* ev = mDependentEvents[i];
     if (ev->IsShow()) {
-      for (uint32_t i = 0; i < aLevel; i++) {
+      for (uint32_t i = 0; i < aLevel + 1; i++) {
         printf("  ");
       }
       logging::AccessibleInfo("shown", ev->mAccessible);
+
+      AccShowEvent* showEv = downcast_accEvent(ev);
+      for (uint32_t i = 0; i < showEv->mPrecedingEvents.Length(); i++) {
+        for (uint32_t j = 0; j < aLevel + 1; j++) {
+          printf("  ");
+        }
+        logging::AccessibleInfo("preceding",
+                                showEv->mPrecedingEvents[i]->mAccessible);
+      }
     }
     else {
-      for (uint32_t i = 0; i < aLevel; i++) {
+      for (uint32_t i = 0; i < aLevel + 1; i++) {
         printf("  ");
       }
       logging::AccessibleInfo("hidden", ev->mAccessible);
     }
   }
 
   if (mFirst) {
     mFirst->Log(aLevel + 1);
--- a/accessible/base/EventTree.h
+++ b/accessible/base/EventTree.h
@@ -62,27 +62,19 @@ class EventTree final {
 public:
   EventTree() :
     mFirst(nullptr), mNext(nullptr), mContainer(nullptr), mFireReorder(false) { }
   explicit EventTree(Accessible* aContainer, bool aFireReorder) :
     mFirst(nullptr), mNext(nullptr), mContainer(aContainer),
     mFireReorder(aFireReorder) { }
   ~EventTree() { Clear(); }
 
-  void Shown(Accessible* aChild)
-  {
-    RefPtr<AccShowEvent> ev = new AccShowEvent(aChild);
-    Mutated(ev);
-  }
+  void Shown(Accessible* aChild);
 
-  void Hidden(Accessible* aChild, bool aNeedsShutdown = true)
-  {
-    RefPtr<AccHideEvent> ev = new AccHideEvent(aChild, aNeedsShutdown);
-    Mutated(ev);
-  }
+  void Hidden(Accessible* aChild, bool aNeedsShutdown = true);
 
   /**
    * Return an event tree node for the given accessible.
    */
   const EventTree* Find(const Accessible* aContainer) const;
 
 #ifdef A11Y_LOG
   void Log(uint32_t aLevel = UINT32_MAX) const;
@@ -104,16 +96,19 @@ private:
 
   UniquePtr<EventTree> mFirst;
   UniquePtr<EventTree> mNext;
 
   Accessible* mContainer;
   nsTArray<RefPtr<AccMutationEvent>> mDependentEvents;
   bool mFireReorder;
 
+  static NotificationController* Controller(Accessible* aAcc)
+    { return aAcc->Document()->Controller(); }
+
   friend class NotificationController;
 };
 
 
 } // namespace a11y
 } // namespace mozilla
 
 #endif // mozilla_a11y_EventQueue_h_
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -21,16 +21,20 @@ using namespace mozilla::a11y;
 // NotificationCollector
 ////////////////////////////////////////////////////////////////////////////////
 
 NotificationController::NotificationController(DocAccessible* aDocument,
                                                nsIPresShell* aPresShell) :
   EventQueue(aDocument), mObservingState(eNotObservingRefresh),
   mPresShell(aPresShell)
 {
+#ifdef DEBUG
+  mMoveGuardOnStack = false;
+#endif
+
   // Schedule initial accessible tree construction.
   ScheduleProcessing();
 }
 
 NotificationController::~NotificationController()
 {
   NS_ASSERTION(!mDocument, "Controller wasn't shutdown properly!");
   if (mDocument)
--- a/accessible/base/NotificationController.h
+++ b/accessible/base/NotificationController.h
@@ -127,16 +127,39 @@ public:
   }
 
   /**
    * Returns existing event tree for the given the accessible or creates one if
    * it doesn't exists yet.
    */
   EventTree* QueueMutation(Accessible* aContainer);
 
+  class MoveGuard final {
+  public:
+    explicit MoveGuard(NotificationController* aController) :
+      mController(aController)
+    {
+#ifdef DEBUG
+      MOZ_ASSERT(!mController->mMoveGuardOnStack,
+                 "Move guard is on stack already!");
+      mController->mMoveGuardOnStack = true;
+#endif
+    }
+    ~MoveGuard() {
+#ifdef DEBUG
+      MOZ_ASSERT(mController->mMoveGuardOnStack, "No move guard on stack!");
+      mController->mMoveGuardOnStack = false;
+#endif
+      mController->mPrecedingEvents.Clear();
+    }
+
+  private:
+    NotificationController* mController;
+  };
+
 #ifdef A11Y_LOG
   const EventTree& RootEventTree() const { return mEventTree; };
 #endif
 
   /**
    * Schedule binding the child document to the tree of this document.
    */
   void ScheduleChildDocBinding(DocAccessible* aDocument);
@@ -242,16 +265,36 @@ protected:
 
 private:
   NotificationController(const NotificationController&);
   NotificationController& operator = (const NotificationController&);
 
   // nsARefreshObserver
   virtual void WillRefresh(mozilla::TimeStamp aTime) override;
 
+  /**
+   * Set and returns a hide event, paired with a show event, for the move.
+   */
+  void WithdrawPrecedingEvents(nsTArray<RefPtr<AccHideEvent>>* aEvs)
+  {
+    if (mPrecedingEvents.Length() > 0) {
+      aEvs->AppendElements(mozilla::Move(mPrecedingEvents));
+    }
+  }
+  void StorePrecedingEvent(AccHideEvent* aEv)
+  {
+    MOZ_ASSERT(mMoveGuardOnStack, "No move guard on stack!");
+    mPrecedingEvents.AppendElement(aEv);
+  }
+  void StorePrecedingEvents(nsTArray<RefPtr<AccHideEvent>>&& aEvs)
+  {
+    MOZ_ASSERT(mMoveGuardOnStack, "No move guard on stack!");
+    mPrecedingEvents.InsertElementsAt(0, aEvs);
+  }
+
 private:
   /**
    * Indicates whether we're waiting on an event queue processing from our
    * notification controller to flush events.
    */
   enum eObservingState {
     eNotObservingRefresh,
     eRefreshObserving,
@@ -315,14 +358,27 @@ private:
    * Holds all scheduled relocations.
    */
   nsTArray<RefPtr<Accessible> > mRelocations;
 
   /**
    * Holds all mutation events.
    */
   EventTree mEventTree;
+
+  /**
+   * A temporary collection of hide events that should be fired before related
+   * show event. Used by EventTree.
+   */
+  nsTArray<RefPtr<AccHideEvent>> mPrecedingEvents;
+
+#ifdef DEBUG
+  bool mMoveGuardOnStack;
+#endif
+
+  friend class MoveGuard;
+  friend class EventTree;
 };
 
 } // namespace a11y
 } // namespace mozilla
 
 #endif // mozilla_a11y_NotificationController_h_
--- a/accessible/base/nsEventShell.cpp
+++ b/accessible/base/nsEventShell.cpp
@@ -14,17 +14,17 @@ using namespace mozilla::a11y;
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsEventShell
 ////////////////////////////////////////////////////////////////////////////////
 
 void
 nsEventShell::FireEvent(AccEvent* aEvent)
 {
-  if (!aEvent)
+  if (!aEvent || aEvent->mEventRule == AccEvent::eDoNotEmit)
     return;
 
   Accessible* accessible = aEvent->GetAccessible();
   NS_ENSURE_TRUE_VOID(accessible);
 
   nsINode* node = accessible->GetNode();
   if (node) {
     sEventTargetNode = node;
@@ -38,16 +38,17 @@ nsEventShell::FireEvent(AccEvent* aEvent
     GetAccService()->GetStringEventType(aEvent->GetEventType(), type);
     logging::MsgEntry("type: %s", NS_ConvertUTF16toUTF8(type).get());
     logging::AccessibleInfo("target", aEvent->GetAccessible());
     logging::MsgEnd();
   }
 #endif
 
   accessible->HandleAccEvent(aEvent);
+  aEvent->mEventRule = AccEvent::eDoNotEmit;
 
   sEventTargetNode = nullptr;
 }
 
 void
 nsEventShell::FireEvent(uint32_t aEventType, Accessible* aAccessible,
                         EIsFromUserInput aIsFromUserInput)
 {
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -2175,16 +2175,18 @@ DocAccessible::MoveChild(Accessible* aCh
 #endif
 
   // If the child was taken from from an ARIA owns element.
   if (aChild->IsRelocated()) {
     nsTArray<RefPtr<Accessible> >* children = mARIAOwnsHash.Get(curParent);
     children->RemoveElement(aChild);
   }
 
+  NotificationController::MoveGuard mguard(mNotificationController);
+
   if (curParent == aNewParent) {
     MOZ_ASSERT(aChild->IndexInParent() != aIdxInParent, "No move case");
     curParent->MoveChild(aIdxInParent, aChild);
 
 #ifdef A11Y_LOG
     logging::TreeInfo("move child: parent tree after",
                       logging::eVerbose, curParent);
 #endif
--- a/accessible/tests/mochitest/events/test_coalescence.html
+++ b/accessible/tests/mochitest/events/test_coalescence.html
@@ -496,17 +496,17 @@
      * <div id="t7_c">
      *   <div id="t7_c_directchild">ha</div>
      *   <div><div id="t7_c_grandchild">ha</div></div>
      * </div>
      */
     function test7()
     {
       this.eventSeq = [
-        new todo_invokerChecker(EVENT_HIDE, getNode('t7_c')),
+        new invokerChecker(EVENT_HIDE, getNode('t7_c')),
         new invokerChecker(EVENT_SHOW, getNode('t7_c')),
         new invokerChecker(EVENT_REORDER, getNode('t7')),
         new unexpectedInvokerChecker(EVENT_REORDER, getNode('t7_c_directchild')),
         new unexpectedInvokerChecker(EVENT_REORDER, getNode('t7_c_grandchild')),
         new unexpectedInvokerChecker(EVENT_SHOW, () => getNode('t7_c_directchild').firstChild),
         new unexpectedInvokerChecker(EVENT_SHOW, () => getNode('t7_c_grandchild').firstChild)
       ];
 
@@ -517,21 +517,57 @@
         getNode('t7_moveplace').setAttribute('aria-owns', 't7_c');
       };
 
       this.getID = function test7_getID() {
         return "Show child accessibles and then hide their container";
       };
     }
 
+    /**
+     * Move a node by aria-owns from right to left in the tree, so that
+     * the eventing looks this way:
+     * reorder for 't8_c1'
+     *   hide for 't8_c1_child'
+     *   show for 't8_c2_moved'
+     * reorder for 't8_c2'
+     *   hide for 't8_c2_moved'
+     *
+     * The hide event should be delivered before the paired show event.
+     */
+    function test8()
+    {
+      this.eventSeq = [
+        new invokerChecker(EVENT_HIDE, getNode('t8_c1_child')),
+        new invokerChecker(EVENT_HIDE, 't8_c2_moved'),
+        new invokerChecker(EVENT_SHOW, 't8_c2_moved'),
+        new invokerChecker(EVENT_REORDER, 't8_c1'),
+        new invokerChecker(EVENT_REORDER, 't8_c2')
+      ];
+
+      this.invoke = function test8_invoke()
+      {
+        // Remove a node from 't8_c1' container to give the event tree a
+        // desired structure (the 't8_c1' container node goes first in the event
+        // tree)
+        getNode('t8_c1_child').remove();
+        // then move 't8_c2_moved' from 't8_c2' to 't8_c1'.
+        getNode('t8_c1').setAttribute('aria-owns', 't8_c2_moved');
+      };
+
+      this.getID = function test8_getID() {
+        return "Move a node by aria-owns to left within the tree";
+      };
+    }
+
     ////////////////////////////////////////////////////////////////////////////
     // Do tests.
 
     //gA11yEventDumpToConsole = true; // debug stuff
-    //enableLogging("tree,eventTree,verbose");
+    //enableLogging("eventTree");
 
     var gQueue = null;
     function doTests()
     {
       gQueue = new eventQueue();
 
       gQueue.push(new removeChildNParent("option1", "select1"));
       gQueue.push(new removeParentNChild("option2", "select2"));
@@ -550,16 +586,17 @@
       gQueue.push(new showParentNAddChild("select12", true));
 
       gQueue.push(new removeGrandChildrenNHideParent("t1_child1", "t1_child2", "t1_parent"));
       gQueue.push(new test3());
       gQueue.push(new test4());
       gQueue.push(new test5());
       gQueue.push(new test6());
       gQueue.push(new test7());
+      gQueue.push(new test8());
 
       gQueue.invoke(); // Will call SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addA11yLoadEvent(doTests);
   </script>
 </head>
@@ -576,17 +613,16 @@
      href="https://bugzilla.mozilla.org/show_bug.cgi?id=570275">
     Mozilla Bug 570275
   </a>
 
   <p id="display"></p>
   <div id="content" style="display: none"></div>
   <pre id="test">
   </pre>
-  <div id="eventdump"></div>
 
   <div id="testContainer">
     <select id="select1">
       <option id="option1">option</option>
     </select>
     <select id="select2">
       <option id="option2">option</option>
     </select>
@@ -651,10 +687,17 @@
 
   <div id="t7">
     <div id="t7_moveplace"></div>
     <div id="t7_c">
       <div><div id="t7_c_grandchild"></div></div>
       <div id="t7_c_directchild"></div>
     </div>
   </div>
+
+  <div id="t8">
+    <div id="t8_c1"><div id="t8_c1_child"></div></div>
+    <div id="t8_c2">
+      <div id="t8_c2_moved"></div>
+    </div>
+  </div>
 </body>
 </html>
--- a/accessible/tests/mochitest/treeupdate/test_ariaowns.html
+++ b/accessible/tests/mochitest/treeupdate/test_ariaowns.html
@@ -68,20 +68,20 @@
       {
         return "Change @aria-owns attribute";
       }
     }
 
     function removeARIAOwns()
     {
       this.eventSeq = [
+        new invokerChecker(EVENT_HIDE, getNode("t1_subdiv")),
         new invokerChecker(EVENT_SHOW, getNode("t1_subdiv")),
         new invokerChecker(EVENT_HIDE, getNode("t1_button")),
         new invokerChecker(EVENT_SHOW, getNode("t1_button")),
-        new invokerChecker(EVENT_HIDE, getNode("t1_subdiv")),
         new invokerChecker(EVENT_REORDER, getNode("t1_container"))
       ];
 
       this.invoke = function removeARIAOwns_invoke()
       {
         getNode("t1_container").removeAttribute("aria-owns");
       }
 
@@ -137,18 +137,18 @@
       {
         return "Set @aria-owns attribute";
       }
     }
 
     function addIdToARIAOwns()
     {
       this.eventSeq = [
+        new invokerChecker(EVENT_HIDE, getNode("t1_group")),
         new invokerChecker(EVENT_SHOW, getNode("t1_group")),
-        new invokerChecker(EVENT_HIDE, getNode("t1_group")),
         new invokerChecker(EVENT_REORDER, document)
       ];
 
       this.invoke = function addIdToARIAOwns_invoke()
       {
         getNode("t1_container").
           setAttribute("aria-owns", "t1_button t1_subdiv t1_group");
       }
@@ -270,18 +270,18 @@
       {
         return "Remove ID from ARIA owned element";
       }
     }
 
     function setId()
     {
       this.eventSeq = [
+        new invokerChecker(EVENT_HIDE, getNode("t1_grouptmp")),
         new invokerChecker(EVENT_SHOW, getNode("t1_grouptmp")),
-        new invokerChecker(EVENT_HIDE, getNode("t1_grouptmp")),
         new invokerChecker(EVENT_REORDER, document)
       ];
 
       this.invoke = function setId_invoke()
       {
         getNode("t1_grouptmp").setAttribute("id", "t1_group");
       }