Bug 230254

Summary: [GTK][a11y] Add a build option to enable ATSPI
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, annulen, aperez, apinheiro, bugs-noreply, cfleizach, dmazzoni, ews-watchlist, gyuyoung.kim, jcraig, jdiggs, mifenton, ryuan.choi, samuel_white, sergio
Priority: P2 Keywords: Gtk
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 230299, 230301    
Bug Blocks: 230253, 230255    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch aperez: review+

Carlos Garcia Campos
Reported 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.
Attachments
Patch (98.53 KB, patch)
2021-09-14 06:07 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (98.03 KB, patch)
2021-09-14 06:27 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (98.49 KB, patch)
2021-09-14 06:33 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (98.95 KB, patch)
2021-09-14 06:41 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (99.47 KB, patch)
2021-09-14 06:47 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (99.47 KB, patch)
2021-09-14 07:00 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (89.06 KB, patch)
2021-09-16 00:36 PDT, Carlos Garcia Campos 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (88.50 KB, patch)
2021-09-16 01:02 PDT, Carlos Garcia Campos 		aperez: review+
DetailsFormatted DiffDiff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2021-09-14 06:07:02 PDT
Created attachment 438125 [details] Patch
Carlos Garcia Campos
Comment 2 2021-09-14 06:27:02 PDT
Created attachment 438127 [details] Patch
Carlos Garcia Campos
Comment 3 2021-09-14 06:33:55 PDT
Created attachment 438128 [details] Patch
Carlos Garcia Campos
Comment 4 2021-09-14 06:41:12 PDT
Created attachment 438130 [details] Patch
Carlos Garcia Campos
Comment 5 2021-09-14 06:47:25 PDT
Created attachment 438131 [details] Patch
Carlos Garcia Campos
Comment 6 2021-09-14 07:00:45 PDT
Created attachment 438134 [details] Patch
Andres Gonzalez
Comment 7 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.
Andres Gonzalez
Comment 8 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.
Andres Gonzalez
Comment 9 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.
Carlos Garcia Campos
Comment 10 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
Carlos Garcia Campos
Comment 11 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.
Carlos Garcia Campos
Comment 12 2021-09-16 00:36:30 PDT
Created attachment 438324 [details] Patch
Carlos Garcia Campos
Comment 13 2021-09-16 01:02:01 PDT
Created attachment 438326 [details] Patch
Adrian Perez
Comment 14 2021-09-17 00:49:54 PDT
Comment on attachment 438326 [details] Patch Approved, as it seems all the review concerns have been adressed :)
Carlos Garcia Campos
Comment 15 2021-09-17 01:41:23 PDT
Committed r282643 (241797@main): <https://commits.webkit.org/241797@main>
Note You need to log in before you can comment on or make changes to this bug.