[webkit-changes] [WebKit/WebKit] c63004: AX: Tree updates become broken when children chang...

[Tyler Wilcock]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: c630047b889cdac6aaabf58285f7454af91bb7e8
      https://github.com/WebKit/WebKit/commit/c630047b889cdac6aaabf58285f7454af91bb7e8
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2024-10-12 (Sat, 12 Oct 2024)

  Changed paths:
    A LayoutTests/accessibility/tree-update-with-dynamically-ignored-inline-child-div-expected.txt
    A LayoutTests/accessibility/tree-update-with-dynamically-ignored-inline-child-div.html
    A LayoutTests/accessibility/tree-update-with-dynamically-ignored-tabindex-div-expected.txt
    A LayoutTests/accessibility/tree-update-with-dynamically-ignored-tabindex-div.html
    M LayoutTests/platform/glib/TestExpectations
    M LayoutTests/platform/ios/TestExpectations
    A LayoutTests/platform/ios/accessibility/tree-update-with-dynamically-ignored-inline-child-div-expected.txt
    A LayoutTests/platform/ios/accessibility/tree-update-with-dynamically-ignored-tabindex-div-expected.txt
    M Source/WebCore/accessibility/AXLogger.cpp
    M Source/WebCore/accessibility/AXObjectCache.h
    M Source/WebCore/accessibility/AccessibilityObject.cpp
    M Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h

  Log Message:
  -----------
  AX: Tree updates become broken when children change for a dynamically ignored element and its unignored ancestor in the same tree update cycle
https://bugs.webkit.org/show_bug.cgi?id=281324
rdar://137765781

Reviewed by Chris Fleizach.

Consider this HTML:

```
<main>
++<div tabindex="0">Foo</div>
</main
```

And this sequence:

  1. Insert one new child to the main and the div

  2. Remove tabindex from the div (causing `AXIsolatedTree::updateNode` for the div, in turn queuing the div in
     `AXIsolatedTree::m_unresolvedPendingAppends`, thus protecting it from being removed from `AXIsolatedTree::m_nodeMap`
     in this tree update cycle). This also means the div will go from being unignored to ignored in the middle of handling
     the children change for it (`AXObjectCache::handleChildrenChanged`)

  3. In `AXIsolatedTree::updateChildren` for the main, we correctly realize that we no longer have the div as a child
     because it is now ignored, and thus try to remove it from the node map. But we protected it in step 2, meaning
     it won't be removed, despite it not "being in the tree" (which is what the node map is supposed to model).

  4. In any subsequent insert into the subtree of the div, we will fail to add said content to the accessibility tree.
     This is because in `AXIsolatedTree::updateChildren`, we use the `m_nodeMap` to iterate up to the nearest "in-tree"
     (unignored) ancestor, and update that. But because we never removed the div from the node map, it will always receive
     the updates, not our main.

We fix this by immediately removing anything becomes ignored from the node map, guaranteeing it can't have improper
effect on children updates that may be in-progress.

Two layout tests are added. These tests only differ in the is-ignored conditions they exercise, ensuring that if we
change or remove one, we don't lose test coverage of this bug.

This patch also includes an unrelated change to add the ability to log AXPropertyMap and AXPropertyName via WTF::TextStream,
as I found this useful when investigating this bug.

* LayoutTests/accessibility/tree-update-with-dynamically-ignored-inline-child-div-expected.txt: Added.
* LayoutTests/accessibility/tree-update-with-dynamically-ignored-inline-child-div.html: Added.
* LayoutTests/accessibility/tree-update-with-dynamically-ignored-tabindex-div-expected.txt: Added.
* LayoutTests/accessibility/tree-update-with-dynamically-ignored-tabindex-div.html: Added.
* LayoutTests/platform/glib/TestExpectations: Skip new tests.
* LayoutTests/platform/ios/TestExpectations: Enable new tests.
* Source/WebCore/accessibility/AXLogger.cpp:
(WebCore::operator<<):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::isIgnoredWithoutCache const):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
(WebCore::AXIsolatedTree::objectBecameIgnored):

Canonical link: https://commits.webkit.org/285073@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications