Bug 235189

Summary: AX: AXIsolatedObject::initializeAttributeData should compute AXAncestorFlags if they are unexpectedly uninitialized
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Tyler Wilcock 2022-01-13 10:01:10 PST
Currently in AXIsolatedObject::initializeAttributeData we have this:

    auto ancestorFlags = object.ancestorFlags();
    // Only store the object's ancestor flags if any are set (excluding the "is initialized" flag).
    if (ancestorFlags ^ AXAncestorFlag::FlagsInitialized)
        setProperty(AXPropertyName::AncestorFlags, object.ancestorFlags());

Instead, we should do the traversal to initialize the flags rather than creating an isolated object with missing properties.
Comment 1 Radar WebKit Bug Importer 2022-01-13 10:01:21 PST
<rdar://problem/87556197>
Comment 2 Tyler Wilcock 2022-01-13 10:11:46 PST
Created attachment 449077 [details]
Patch
Comment 3 Andres Gonzalez 2022-01-13 14:02:56 PST
(In reply to Tyler Wilcock from comment #2)
> Created attachment 449077 [details]
> Patch

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp

+OptionSet<AXAncestorFlag> AccessibilityObject::computeAncestorFlagsWithTraversal() const
+{
+    OptionSet<AXAncestorFlag> computedFlags;
+    computedFlags.set(AXAncestorFlag::FlagsInitialized, true);

Shouldn't we check that it hasn't been initialized yet? At least an ASSERT to catch it on debug?

+    Accessibility::enumerateAncestors<AXCoreObject>(*this, false, [&] (const AXCoreObject& ancestor) {
+        if (is<AccessibilityObject>(ancestor))
+            computedFlags.add(downcast<AccessibilityObject>(ancestor).computeAncestorFlags());

Can't we use AccessibilityObject as template parameter? Or let the compiler infer the parameter? In either case, I don't think you would need is<> and the downcast.
Comment 4 Tyler Wilcock 2022-01-13 15:25:18 PST
Created attachment 449120 [details]
Patch
Comment 5 Tyler Wilcock 2022-01-13 15:25:45 PST
Good points. Fixed both issues with the latest patch.
Comment 6 EWS 2022-01-14 13:05:26 PST
Committed r288027 (246053@main): <https://commits.webkit.org/246053@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449120 [details].