Bug 235189 - AX: AXIsolatedObject::initializeAttributeData should compute AXAncestorFlags if they are unexpectedly uninitialized
Summary: AX: AXIsolatedObject::initializeAttributeData should compute AXAncestorFlags ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
Keywords: InRadar
Depends on:
Reported: 2022-01-13 10:01 PST by Tyler Wilcock
Modified: 2022-01-14 13:05 PST (History)
10 users (show)

See Also:

Patch (6.21 KB, patch)
2022-01-13 10:11 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (6.29 KB, patch)
2022-01-13 15:25 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
Comment 2 Tyler Wilcock 2022-01-13 10:11:46 PST
Created attachment 449077 [details]
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]
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].