[webkit-changes] [WebKit/WebKit] 4b39d6: AX: accessibility/mac/label-element-all-text-strin...

[Tyler Wilcock]

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

  Changed paths:
    M Source/WebCore/accessibility/AccessibilityLabel.cpp
    M Source/WebCore/accessibility/AccessibilityLabel.h
    M Source/WebCore/accessibility/AccessibilityObject.h

  Log Message:
  -----------
  AX: accessibility/mac/label-element-all-text-string-value.html and two other label-related tests fail in ENABLE(INCLUDE_IGNORED_IN_CORE_AX_TREE)
https://bugs.webkit.org/show_bug.cgi?id=280568
rdar://problem/136894363

Reviewed by Chris Fleizach.

These tests fail in ENABLE(INCLUDE_IGNORED_IN_CORE_AX_TREE) because in https://commits.webkit.org/284418@main, we replaced
direct usage of `m_children` with a call to `unignoredChildren(/* updateChildrenIfNecessary */ false)` in AccessibilityLabel::updateChildrenIfNecessary()
with the assumption that this would result in the same behavior. However, that assumption is not true, as without updating
the children of some descendants (because of the transitively passed `/* updateChildrenIfNecessary */ false` value),
AXCoreObject::nextInPreOrder does not return the right objects to compute AccessibilityLabel::containsOnlyStaticText
correctly.

Avoid this problem by dirtying `AccessibilityLabel::m_containsOnlyStaticTextDirty` once in a new `AccessibilityLabel::addChildren`
override, and removing the `updateChildrenIfNecessary` and `insertChild` overrides. This re-ordering allows us to safely
call update-children form of `unignoredChildren()` when requests in `containsOnlyStaticText`.

This is also more performant in a couple ways:

  1. We only resolve `containsOnlyStaticText` when it's actually
     requested rather than eagerly
  2. `AccessibilityLabel` was the only overrider of `AccessibilityObject::insertChild`, so removing it
     allows us to make `insertChild` non-virtual. This is actually pretty significant as it's one of the most called
     functions that we have.

We may need to audit other usages of `unignoredChildren(/* updateChildrenIfNecessary */ false)` added in
https://commits.webkit.org/284418@main as a follow-up to this patch.

* Source/WebCore/accessibility/AccessibilityLabel.cpp:
(WebCore::childrenContainOnlyStaticText):
(WebCore::AccessibilityLabel::containsOnlyStaticText const):
(WebCore::AccessibilityLabel::addChildren):
(WebCore::AccessibilityLabel::computeIsIgnored const): Deleted.
(WebCore::AccessibilityLabel::updateChildrenIfNecessary): Deleted.
(WebCore::AccessibilityLabel::insertChild): Deleted.
* Source/WebCore/accessibility/AccessibilityLabel.h:
* Source/WebCore/accessibility/AccessibilityObject.h:

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



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