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 315886 810a4f6dff0407c622e89cea3a6bb9a532c01cd0
parent 315885 e5049535533e2af84066c0ad7e2e2259b123d8ee
child 315887 93a20b9fe4f9716bf5a76e89e65cdb15d79585b4
push id30757
push usercbook@mozilla.com
push dateFri, 30 Sep 2016 10:02:43 +0000
treeherdermozilla-central@5ffed033557e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1294853
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");
       }