Bug 230258 – [GTK][a11y] Add implementation of text interface when building with ATSPI

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

Implement text interface

[Carlos Garcia Campos]

Created attachment 441860 [details]
Patch

[Carlos Garcia Campos]

Created attachment 441869 [details]
Patch

[Andres Gonzalez]

(In reply to Carlos Garcia Campos from comment #2)
> Created attachment 441869 [details]
> Patch

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

@@ -1428,9 +1428,10 @@ void AXObjectCache::postTextStateChangeNotification(const Position& position, co

     stopCachingComputedObjectAttributes();

-#if PLATFORM(COCOA)
+#if PLATFORM(COCOA) || USE(ATSPI)
     AccessibilityObject* object = getOrCreate(node);
     if (object && object->accessibilityIsIgnored()) {
+#if PLATFORM(COCOA)
         if (position.atLastEditingPositionForNode()) {
             if (AccessibilityObject* nextSibling = object->nextSiblingUnignored(1))
                 object = nextSibling;
@@ -1438,6 +1439,11 @@ void AXObjectCache::postTextStateChangeNotification(const Position& position, co
             if (AccessibilityObject* previousSibling = object->previousSiblingUnignored(1))
                 object = previousSibling;
         }
+#elif USE(ATSPI)
+        auto* parent = object->parentObjectUnignored();
+        if (is<AccessibilityObject>(parent))
+            object = downcast<AccessibilityObject>(parent);
+#endif
     }

     postTextStateChangeNotification(object, intent, selection);

Why the target should be parentObjectUnignored and not the prev/next sibling?

[Carlos Garcia Campos]

(In reply to Andres Gonzalez from comment #3)
> (In reply to Carlos Garcia Campos from comment #2)
> > Created attachment 441869 [details]
> > Patch
> 
> --- a/Source/WebCore/accessibility/AXObjectCache.cpp
> +++ a/Source/WebCore/accessibility/AXObjectCache.cpp
> 
> @@ -1428,9 +1428,10 @@ void
> AXObjectCache::postTextStateChangeNotification(const Position& position, co
> 
>      stopCachingComputedObjectAttributes();
> 
> -#if PLATFORM(COCOA)
> +#if PLATFORM(COCOA) || USE(ATSPI)
>      AccessibilityObject* object = getOrCreate(node);
>      if (object && object->accessibilityIsIgnored()) {
> +#if PLATFORM(COCOA)
>          if (position.atLastEditingPositionForNode()) {
>              if (AccessibilityObject* nextSibling =
> object->nextSiblingUnignored(1))
>                  object = nextSibling;
> @@ -1438,6 +1439,11 @@ void
> AXObjectCache::postTextStateChangeNotification(const Position& position, co
>              if (AccessibilityObject* previousSibling =
> object->previousSiblingUnignored(1))
>                  object = previousSibling;
>          }
> +#elif USE(ATSPI)
> +        auto* parent = object->parentObjectUnignored();
> +        if (is<AccessibilityObject>(parent))
> +            object = downcast<AccessibilityObject>(parent);
> +#endif
>      }
> 
>      postTextStateChangeNotification(object, intent, selection);
> 
> Why the target should be parentObjectUnignored and not the prev/next sibling?

Because in linux we don't expose text nodes in the tree, so it's the parent the one implementing the text interface.

[Andres Gonzalez]

(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Andres Gonzalez from comment #3)
> > (In reply to Carlos Garcia Campos from comment #2)
> > > Created attachment 441869 [details]
> > > Patch
> > 
> > --- a/Source/WebCore/accessibility/AXObjectCache.cpp
> > +++ a/Source/WebCore/accessibility/AXObjectCache.cpp
> > 
> > @@ -1428,9 +1428,10 @@ void
> > AXObjectCache::postTextStateChangeNotification(const Position& position, co
> > 
> >      stopCachingComputedObjectAttributes();
> > 
> > -#if PLATFORM(COCOA)
> > +#if PLATFORM(COCOA) || USE(ATSPI)
> >      AccessibilityObject* object = getOrCreate(node);
> >      if (object && object->accessibilityIsIgnored()) {
> > +#if PLATFORM(COCOA)
> >          if (position.atLastEditingPositionForNode()) {
> >              if (AccessibilityObject* nextSibling =
> > object->nextSiblingUnignored(1))
> >                  object = nextSibling;
> > @@ -1438,6 +1439,11 @@ void
> > AXObjectCache::postTextStateChangeNotification(const Position& position, co
> >              if (AccessibilityObject* previousSibling =
> > object->previousSiblingUnignored(1))
> >                  object = previousSibling;
> >          }
> > +#elif USE(ATSPI)
> > +        auto* parent = object->parentObjectUnignored();
> > +        if (is<AccessibilityObject>(parent))
> > +            object = downcast<AccessibilityObject>(parent);
> > +#endif
> >      }
> > 
> >      postTextStateChangeNotification(object, intent, selection);
> > 
> > Why the target should be parentObjectUnignored and not the prev/next sibling?
> 
> Because in linux we don't expose text nodes in the tree, so it's the parent
> the one implementing the text interface.

Thanks, makes sense. A comment in the code stating this would be helpful.

[Carlos Garcia Campos]

Committed r284675 (243395@main): <https://commits.webkit.org/243395@main>

[Michael Catanzaro]

It introduced this warning:

[824/1963] Building CXX object Source/...sources/UnifiedSource-aba958d6-1.cpp.o
In file included from /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME/WebCore/DerivedSources/unified-sources/UnifiedSource-aba958d6-1.cpp:3:
/home/mcatanzaro/Projects/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp:1404:13: warning: ‘bool WebCore::isPasswordFieldOrContainedByPasswordField(WebCore::AccessibilityObject*)’ defined but not used [-Wunused-function]
 1404 | static bool isPasswordFieldOrContainedByPasswordField(AccessibilityObject* object)
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can you check the #if guards, please? It's easy enough to guard the newly-unused function, but I'm not sure whether your change in AXObjectCache::enqueuePasswordValueChangeNotification was intentional.