Bug 230254 - [GTK][a11y] Add a build option to enable ATSPI
Summary: [GTK][a11y] Add a build option to enable ATSPI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 230299 230301
Blocks: ATSPI 230255
  Show dependency treegraph
 
Reported: 2021-09-14 05:31 PDT by Carlos Garcia Campos
Modified: 2021-09-17 01:41 PDT (History)
16 users (show)

See Also:


Attachments
Patch (98.53 KB, patch)
2021-09-14 06:07 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (98.03 KB, patch)
2021-09-14 06:27 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (98.49 KB, patch)
2021-09-14 06:33 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (98.95 KB, patch)
2021-09-14 06:41 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (99.47 KB, patch)
2021-09-14 06:47 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (99.47 KB, patch)
2021-09-14 07:00 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (89.06 KB, patch)
2021-09-16 00:36 PDT, Carlos Garcia Campos
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (88.50 KB, patch)
2021-09-16 01:02 PDT, Carlos Garcia Campos
aperez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2021-09-14 05:31:27 PDT
Add USE_ATSPI build option that disables ATK and enables the new atspi based implementation, for now just adding stubs and build fixes with isolated tree enabled.
Comment 1 Carlos Garcia Campos 2021-09-14 06:07:02 PDT
Created attachment 438125 [details]
Patch
Comment 2 Carlos Garcia Campos 2021-09-14 06:27:02 PDT
Created attachment 438127 [details]
Patch
Comment 3 Carlos Garcia Campos 2021-09-14 06:33:55 PDT
Created attachment 438128 [details]
Patch
Comment 4 Carlos Garcia Campos 2021-09-14 06:41:12 PDT
Created attachment 438130 [details]
Patch
Comment 5 Carlos Garcia Campos 2021-09-14 06:47:25 PDT
Created attachment 438131 [details]
Patch
Comment 6 Carlos Garcia Campos 2021-09-14 07:00:45 PDT
Created attachment 438134 [details]
Patch
Comment 7 Andres Gonzalez 2021-09-14 17:38:46 PDT
(In reply to Carlos Garcia Campos from comment #6)
> Created attachment 438134 [details]
> Patch

--- a/Source/WebCore/accessibility/AXObjectCache.cpp
+++ a/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -770,9 +770,13 @@ AccessibilityObject* AXObjectCache::getOrCreate(RenderObject* renderer)
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
 bool AXObjectCache::clientSupportsIsolatedTree()
 {
+#if USE(APPLE_INTERNAL_SDK)

Instead of adding more #if throughout the platform-independent source, it would be cleaner to move this mac implementation to AXObjectCacheMac.mm, and add a new implementation for atspi.

Same comment for AXObjectCache::isIsolatedTreeEnabled, initializeSecondaryAXThread and usedOnAXThread.
Comment 8 Andres Gonzalez 2021-09-14 17:42:58 PDT
(In reply to Carlos Garcia Campos from comment #6)
> Created attachment 438134 [details]
> Patch

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -3782,7 +3782,14 @@ static bool isAccessibilityTextSearchMatch(AXCoreObject* axObject, Accessibility
 {
     if (!axObject)
         return false;
-    return axObject->containsText(criteria.searchText);
+
+    // If text is empty we return true.
+    if (criteria.searchText.isEmpty())
+        return true;
+
+    return containsPlainText(axObject->title(), criteria.searchText, CaseInsensitive)
+        || containsPlainText(axObject->accessibilityDescription(), criteria.searchText, CaseInsensitive)
+        || containsPlainText(axObject->stringValue(), criteria.searchText, CaseInsensitive);
 }

This doesn't seem related to enabling isolated tree, so perhaps you can submit this change as a separate patch.
Comment 9 Andres Gonzalez 2021-09-14 17:56:20 PDT
(In reply to Carlos Garcia Campos from comment #6)
> Created attachment 438134 [details]
> Patch

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -71,11 +71,15 @@ void AXIsolatedObject::initializeAttributeData(AXCoreObject& object, bool isRoot
     setProperty(AXPropertyName::ARIALandmarkRoleDescription, object.ariaLandmarkRoleDescription().isolatedCopy());
     setProperty(AXPropertyName::AccessibilityDescription, object.accessibilityDescription().isolatedCopy());
     setProperty(AXPropertyName::BoundingBoxRect, object.boundingBoxRect());
+#if PLATFORM(COCOA)
     setProperty(AXPropertyName::Description, object.descriptionAttributeValue().isolatedCopy());
+#endif

Could we move these platform-specific properties to initializePlatformProperties? That way we wouldn't have to #if throughout here.
Comment 10 Carlos Garcia Campos 2021-09-15 00:26:47 PDT
(In reply to Andres Gonzalez from comment #7)
> (In reply to Carlos Garcia Campos from comment #6)
> > Created attachment 438134 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/AXObjectCache.cpp
> +++ a/Source/WebCore/accessibility/AXObjectCache.cpp
> @@ -770,9 +770,13 @@ AccessibilityObject*
> AXObjectCache::getOrCreate(RenderObject* renderer)
>  #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
>  bool AXObjectCache::clientSupportsIsolatedTree()
>  {
> +#if USE(APPLE_INTERNAL_SDK)
> 
> Instead of adding more #if throughout the platform-independent source, it
> would be cleaner to move this mac implementation to AXObjectCacheMac.mm, and
> add a new implementation for atspi.
> 
> Same comment for AXObjectCache::isIsolatedTreeEnabled,
> initializeSecondaryAXThread and usedOnAXThread.

Ok
Comment 11 Carlos Garcia Campos 2021-09-15 00:29:02 PDT
(In reply to Andres Gonzalez from comment #8)
> (In reply to Carlos Garcia Campos from comment #6)
> > Created attachment 438134 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/AccessibilityObject.cpp
> +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
> @@ -3782,7 +3782,14 @@ static bool
> isAccessibilityTextSearchMatch(AXCoreObject* axObject, Accessibility
>  {
>      if (!axObject)
>          return false;
> -    return axObject->containsText(criteria.searchText);
> +
> +    // If text is empty we return true.
> +    if (criteria.searchText.isEmpty())
> +        return true;
> +
> +    return containsPlainText(axObject->title(), criteria.searchText,
> CaseInsensitive)
> +        || containsPlainText(axObject->accessibilityDescription(),
> criteria.searchText, CaseInsensitive)
> +        || containsPlainText(axObject->stringValue(), criteria.searchText,
> CaseInsensitive);
>  }
> 
> This doesn't seem related to enabling isolated tree, so perhaps you can
> submit this change as a separate patch.

Right, this is weird here, The reason of this change is because WTR uses the WebCore wrappers, and including TextIterator.h from the header pulled too many things that caused linking errors. I'll split the patch.
Comment 12 Carlos Garcia Campos 2021-09-16 00:36:30 PDT
Created attachment 438324 [details]
Patch
Comment 13 Carlos Garcia Campos 2021-09-16 01:02:01 PDT
Created attachment 438326 [details]
Patch
Comment 14 Adrian Perez 2021-09-17 00:49:54 PDT
Comment on attachment 438326 [details]
Patch

Approved, as it seems all the review concerns have been adressed :)
Comment 15 Carlos Garcia Campos 2021-09-17 01:41:23 PDT
Committed r282643 (241797@main): <https://commits.webkit.org/241797@main>