[Webkit-unassigned] [Bug 242401] AX: Expose suggestion, insertion, deletion roles and attributes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=242401

--- Comment #5 from Andres Gonzalez <andresg_22 at apple.com> ---
(In reply to Joshua Hoffman from comment #2)
> Created attachment 460721 [details]
> Patch

--- a/Source/WebCore/accessibility/AccessibilityObjectInterface.h
+++ a/Source/WebCore/accessibility/AccessibilityObjectInterface.h

@@ -503,6 +504,8 @@ ALWAYS_INLINE String accessibilityRoleToString(AccessibilityRole role)
         return "StaticText"_s;
     case AccessibilityRole::Subscript:
         return "Subscript"_s;
+    case AccessibilityRole::Suggestion:
+            return "Suggestion"_s;

Wrong indentation. Style checker should've gotten it.

--- a/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm
+++ a/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm

+- (BOOL)accessibilityIsInsertion
+{
+    if (![self _prepareAccessibilityCall])
+        return nil;

nil -> NO

+    for (AXCoreObject* obj = self.axBackingObject->parentObject(); obj; obj = obj->parentObject()) {

You can probably replace this for loop with a Accessibility::findAncestor. But if you keep the for loop:

Change AXCoreObject* obj -> auto* object

+        if (obj) {

It is unnecessary since it is the condition in the for statement.

+            if (obj->hasTagName(bodyTag))
+                return NO;

You can simply assume that if parentObject() is null you have reached the root of the tree, this is a rather expensive way of checking if you got to the root.

In short, I think you can re-write this as:

return Accessibility::findAncestor(*self.backingObject, true, [] (const auto& object) {
   return object.roleValue() == AccessibilityRole::Insertion;
});

+- (BOOL)accessibilityIsDeletion

Same comments as above apply here.

Further more, these methods may belong to AXCoreObject, in which case, in the platform wrapper you should only need to do 

return self.axBackingObject->isInsertion();

Some of the comments above apply also to the following methods:

+- (BOOL)accessibilityIsFirstItemInSuggestion

and

+- (BOOL)accessibilityIsLastItemInSuggestion

--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
+++ a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
@@ -860,6 +860,14 @@ static void AXAttributeStringSetStyle(NSMutableAttributedString* attrString, Ren

+        if (auto* axObject = node->document().axObjectCache()->getOrCreate(node)) {

axObjectCache() can be null.

+            if (axObject->roleValue() == AccessibilityRole::Insertion)
+                AXAttributeStringSetNumber(attrString, @"AXIsSuggestedInsertion", @YES, range);
+            else if (axObject->roleValue() == AccessibilityRole::Deletion)
+                AXAttributeStringSetNumber(attrString, @"AXIsSuggestedDeletion", @YES, range);
+            if (axObject->roleValue() == AccessibilityRole::Suggestion)
+                AXAttributeStringSetNumber(attrString, @"AXIsSuggestion", @YES, range);

You may want to use a switch here, or the last if should be an else if.

-    if (backingObject->isTextControl()) {
+    if (backingObject->isTextControl() || backingObject->isStaticText()) {

Why we need to change this. I think some of these properties apply to text controls but not to static text.

--- a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
+++ a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm

The four new methods in the AccessibilityUIElement class return NO. Are we testing this on the Mac?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220706/fabe68d7/attachment.htm>