Bug 1919087 - Defer PushNameOrDescriptionChange to WillRefresh tick. r=Jamie

Since the method is deferred we need to do extra guesswork for possible
situtations where the name has changed because we don't have the
privilege to calculate the name in-line when content is deleted.

I tried to account for all cases as we have in our test coverage. I
hope that if there are edge cases they are false positives, and we are
firing extra name changes and not the opposite.

Differential Revision: https://phabricator.services.mozilla.com/D223877
This commit is contained in:
Eitan Isaacson 2024-10-17 04:46:20 +00:00
parent 66067704f4
commit 4ea553fb6c
4 changed files with 80 additions and 26 deletions

View File

@ -51,6 +51,24 @@ bool EventQueue::PushEvent(AccEvent* aEvent) {
return true;
}
bool EventQueue::PushNameOrDescriptionChangeToRelations(
LocalAccessible* aAccessible, RelationType aType) {
MOZ_ASSERT(aType == RelationType::LABEL_FOR ||
aType == RelationType::DESCRIPTION_FOR);
bool pushed = false;
uint32_t eventType = aType == RelationType::LABEL_FOR
? nsIAccessibleEvent::EVENT_NAME_CHANGE
: nsIAccessibleEvent::EVENT_DESCRIPTION_CHANGE;
Relation rel = aAccessible->RelationByType(aType);
while (LocalAccessible* relTarget = rel.LocalNext()) {
RefPtr<AccEvent> nameChangeEvent = new AccEvent(eventType, relTarget);
pushed |= PushEvent(nameChangeEvent);
}
return pushed;
}
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
@ -58,14 +76,18 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
// subtree was changed.
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.
// event we get is text inserted on the container. Or, a reorder event may
// change the container's name. 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) &&
aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_INSERTED ||
aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_REORDER ||
aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_INNER_REORDER) &&
nsTextEquivUtils::HasNameRule(target, eNameFromSubtreeRule);
const bool doName = target->HasNameDependent() || maybeTargetNameChanged;
const bool doDesc = target->HasDescriptionDependent();
if (!doName && !doDesc) {
return false;
}
@ -80,12 +102,34 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
if (doName) {
if (nameCheckAncestor && (maybeTargetNameChanged || parent != target) &&
nsTextEquivUtils::HasNameRule(parent, eNameFromSubtreeRule)) {
nsAutoString name;
ENameValueFlag nameFlag = parent->Name(name);
// If name is obtained from subtree, fire name change event.
// HTML file inputs always get part of their name from the subtree, even
// if the author provided a name.
if (nameFlag == eNameFromSubtree || parent->IsHTMLFileInput()) {
bool fireNameChange = parent->IsHTMLFileInput();
if (!fireNameChange) {
nsAutoString name;
ENameValueFlag nameFlag = parent->Name(name);
switch (nameFlag) {
case eNameOK:
// Descendants of subtree may have been removed, making the name
// void.
fireNameChange = name.IsVoid();
break;
case eNameFromSubtree:
// If name is obtained from subtree, fire name change event.
fireNameChange = true;
break;
case eNameFromTooltip:
// If the descendants of this accessible were removed, the name
// may be calculated using the tooltip. We can guess that the name
// was obtained from the subtree before.
fireNameChange = true;
break;
default:
MOZ_ASSERT_UNREACHABLE("All name flags not covered!");
}
}
if (fireNameChange) {
RefPtr<AccEvent> nameChangeEvent =
new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, parent);
pushed |= PushEvent(nameChangeEvent);
@ -93,21 +137,13 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
nameCheckAncestor = false;
}
Relation rel = parent->RelationByType(RelationType::LABEL_FOR);
while (LocalAccessible* relTarget = rel.LocalNext()) {
RefPtr<AccEvent> nameChangeEvent =
new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, relTarget);
pushed |= PushEvent(nameChangeEvent);
}
pushed |= PushNameOrDescriptionChangeToRelations(parent,
RelationType::LABEL_FOR);
}
if (doDesc) {
Relation rel = parent->RelationByType(RelationType::DESCRIPTION_FOR);
while (LocalAccessible* relTarget = rel.LocalNext()) {
RefPtr<AccEvent> descChangeEvent = new AccEvent(
nsIAccessibleEvent::EVENT_DESCRIPTION_CHANGE, relTarget);
pushed |= PushEvent(descChangeEvent);
}
pushed |= PushNameOrDescriptionChangeToRelations(
parent, RelationType::DESCRIPTION_FOR);
}
if (parent->IsDoc()) {

View File

@ -29,6 +29,9 @@ class EventQueue {
*/
bool PushEvent(AccEvent* aEvent);
bool PushNameOrDescriptionChangeToRelations(LocalAccessible* aAccessible,
RelationType aType);
/**
* Puts name and/or description change events into the queue, if needed.
*/

View File

@ -203,6 +203,20 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
}
}
if (aEvent->GetEventType() == nsIAccessibleEvent::EVENT_HIDE ||
aEvent->GetEventType() == nsIAccessibleEvent::EVENT_SHOW) {
LocalAccessible* target = aEvent->GetAccessible();
// We need to do this here while the relation is still intact. During the
// tick, where we we call PushNameOrDescriptionChange, it will be too late
// since we will already have unparented the label and severed the relation.
if (PushNameOrDescriptionChangeToRelations(target,
RelationType::LABEL_FOR) ||
PushNameOrDescriptionChangeToRelations(target,
RelationType::DESCRIPTION_FOR)) {
ScheduleProcessing();
}
}
// We need to fire a reorder event after all of the events targeted at shown
// 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
@ -213,12 +227,6 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
reorder = new AccReorderEvent(container);
container->SetReorderEventTarget(true);
mMutationMap.PutEvent(reorder);
// Since this is the first child of container that is changing, the name
// and/or description of dependent Accessibles may be changing.
if (PushNameOrDescriptionChange(aEvent)) {
ScheduleProcessing();
}
} else {
AccReorderEvent* event = downcast_accEvent(
mMutationMap.GetEvent(container, EventMap::ReorderEvent));
@ -651,6 +659,13 @@ void NotificationController::ProcessMutationEvents() {
return;
}
// The mutation in the container can change its name, or an ancestor's
// name. A labelled/described by relation would also need to be notified
// if this is the case.
if (PushNameOrDescriptionChange(event)) {
ScheduleProcessing();
}
LocalAccessible* target = event->GetAccessible();
target->Document()->MaybeNotifyOfValueChange(target);
if (!mDocument) {

View File

@ -342,7 +342,7 @@ function testNameForSubtreeRule(aElm) {
if (gDumpToConsole) {
dump(
"\nProcessed from subtree rule. Wait for reorder event on " +
"\nProcessed from subtree rule. Wait for name change event on " +
prettyName(aElm) +
"\n"
);