Bug 1823294: Make PushNameOrDescriptionChange fire a name change event on the target itself if appropriate. r=eeejay

Previously, PushNameOrDescriptionChange wouldn't consider the target itself as a candidate for a name change event.
If the text of a text leaf changes without replacing the leaf, the only event we will get is text inserted on its parent.
If that parent calculated its name from its subtree, this meant that we wouldn't fire a name change in this case.
We address this by allowing this function to fire on the target itself in this case.

Differential Revision: https://phabricator.services.mozilla.com/D172953

accessible/base/EventQueue.cpp

@@ -49,18 +49,26 @@ bool EventQueue::PushEvent(AccEvent* aEvent) {
       (aEvent->mEventType == nsIAccessibleEvent::EVENT_NAME_CHANGE ||
        aEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_REMOVED ||
        aEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_INSERTED)) {
-    PushNameOrDescriptionChange(aEvent->mAccessible);
+    PushNameOrDescriptionChange(aEvent);
   }
   return true;
 }
 
-bool EventQueue::PushNameOrDescriptionChange(LocalAccessible* aTarget) {
+bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
   // Fire name/description change event on parent or related LocalAccessible
   // being labelled/described given that this event hasn't been coalesced, the
   // dependent's name/description was calculated from this subtree, and the
   // subtree was changed.
-  const bool doName = aTarget->HasNameDependent();
-  const bool doDesc = aTarget->HasDescriptionDependent();
+  LocalAccessible* target = aOrigEvent->mAccessible;
+  // If the text of a text leaf changed without replacing the leaf, the only
+  // event we get is text inserted on the container. In this case, we might
+  // need to fire a name change event on the target itself.
+  const bool maybeTargetNameChanged =
+      (aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_REMOVED ||
+       aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_INSERTED) &&
+      nsTextEquivUtils::HasNameRule(target, eNameFromSubtreeRule);
+  const bool doName = target->HasNameDependent() || maybeTargetNameChanged;
+  const bool doDesc = target->HasDescriptionDependent();
   if (!doName && !doDesc) {
     return false;
   }
@@ -69,11 +77,11 @@ bool EventQueue::PushNameOrDescriptionChange(LocalAccessible* aTarget) {
   // Only continue traversing up the tree if it's possible that the parent
   // LocalAccessible's name (or a LocalAccessible being labelled by this
   // LocalAccessible or an ancestor) can depend on this LocalAccessible's name.
-  LocalAccessible* parent = aTarget;
+  LocalAccessible* parent = target;
   do {
     // Test possible name dependent parent.
     if (doName) {
-      if (nameCheckAncestor && parent != aTarget &&
+      if (nameCheckAncestor && (maybeTargetNameChanged || parent != target) &&
           nsTextEquivUtils::HasNameRule(parent, eNameFromSubtreeRule)) {
         nsAutoString name;
         ENameValueFlag nameFlag = parent->Name(name);

accessible/base/EventQueue.h

@@ -28,7 +28,7 @@ class EventQueue {
   /**
    * Puts name and/or description change events into the queue, if needed.
    */
-  bool PushNameOrDescriptionChange(LocalAccessible* aTarget);
+  bool PushNameOrDescriptionChange(AccEvent* aOrigEvent);
 
   /**
    * Process events from the queue and fires events.

accessible/base/NotificationController.cpp

@@ -185,7 +185,6 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
   // or hidden children of a container.  So either queue a new one, or move an
   // existing one to the end of the queue if the container already has a
   // reorder event.
-  LocalAccessible* target = aEvent->GetAccessible();
   LocalAccessible* container = aEvent->GetAccessible()->LocalParent();
   RefPtr<AccReorderEvent> reorder;
   if (!container->ReorderEventTarget()) {
@@ -195,7 +194,7 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
 
     // Since this is the first child of container that is changing, the name
     // and/or description of dependent Accessibles may be changing.
-    if (PushNameOrDescriptionChange(target)) {
+    if (PushNameOrDescriptionChange(aEvent)) {
       ScheduleProcessing();
     }
   } else {
@@ -231,6 +230,7 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
     return true;
   }
 
+  LocalAccessible* target = aEvent->GetAccessible();
   int32_t offset = container->AsHyperText()->GetChildOffset(target);
   AccTreeMutationEvent* prevEvent = aEvent->PrevEvent();
   while (prevEvent &&

accessible/tests/mochitest/events/test_namechange.html

@@ -132,10 +132,18 @@
       await nameChanged;
 
       nameChanged = PromEvents.waitForEvent(EVENT_NAME_CHANGE, "listitem");
-      info("Changing text of listitem child");
+      info("Changing textContent of listitem child");
+      // Changing textContent replaces the text leaf with a new one.
       getNode("listitem").textContent = "world";
       await nameChanged;
 
+      nameChanged = PromEvents.waitForEvent(EVENT_NAME_CHANGE, "button");
+      info("Changing text of button's text leaf");
+      // Changing the text node's data changes the text without replacing the
+      // leaf.
+      getNode("button").firstChild.data = "after";
+      await nameChanged;
+
       SimpleTest.finish();
     }
 
@@ -170,6 +178,8 @@
 
   <ul><li id="listitem">hello</li></ul>
 
+  <button id="button">before</button>
+
   <div id="eventdump"></div>
 </body>
 </html>