[wpe-2.38] atk_object_get_name returns empty string on focus event

[varumugam123]

Version : wpe-2.38 (39d3d180b219aba9fb12e289c0cadfcbf8c6ef7f) Problem : We use our custom Assistive Technology implementation that relies on WebCore/accessibility/atk implementation and installs hooks like this to get notified on focus change event

atk_add_global_event_listener(stateEventListener, "Atk:AtkObject:state-change")

gboolean stateEventListener(GSignalInvocationHint *signal, guint param_count, const GValue *params, gpointer data)
{
    AtkObject *accObj = ATK_OBJECT(g_value_get_object(&params[0]));
    const gchar *propName = g_value_get_string(&params[1]);
    guint d1 = g_value_get_boolean(&params[2]) ? 1 : 0;

    handleEvent(accObj, STATE_CHANGED, propName, d1);

    return TRUE;
}

void handleEvent(AtkObject *obj, const gchar* major, const gchar* minor, guint32 d1)
{
    if(major == "state-changed") {
        if(minor == "focused" && d1 == 1) {
            char *name = atk_object_get_name(obj);
            if(name) {
                printf("Read accessible name : %s\n", name);
            } else {
                printf("Couldn't read accessible name");
            }
        }
    }
}

With this we observe that atk_object_get_name returns an empty string. This behavior is observed with wpe-2.38 only. With wpe-2.28 the same API (and Assistive Technology implementation), valid values are returned.

Sample app that reproduces the issue : issue-with-focus-change.html.txt

Analysis:

• Empty value is returned as during webkitAccessibleGetName the WebkitAccessible object seems detached from AccessibilityObject (aka AXCoreObject)
• This is because, when webkitAccessibleGetName tries to retrieve the AccessibilityObject through AXObjectCache::get(Node*), the get() function sees that it now has a renderer but it doesn't have a valid renderID (refer below code snippet), hence the WebkitAccessible is removed (and in turn detached from AccessibilityObject).

AccessibilityObject* AXObjectCache::get(Node* node)
{
    if (!node)
        return nullptr;

    AXID renderID = node->renderer() ? m_renderObjectMapping.get(node->renderer()) : AXID();
    ASSERT(!renderID.isHashTableDeletedValue());

    AXID nodeID = m_nodeObjectMapping.get(node);
    ASSERT(!nodeID.isHashTableDeletedValue());

    if (node->renderer() && nodeID && !renderID) {
        // This can happen if an AccessibilityNodeObject is created for a node that's not
        // rendered, but later something changes and it gets a renderer (like if it's
        // reparented).
        remove(nodeID);
        return nullptr;
    }

    if (renderID)
        return m_objects.get(renderID);

    if (!nodeID)
        return nullptr;

    return m_objects.get(nodeID);
}

• The reason for this behavior (not having a renderer at the time of creating WebKitAccessible object and associating one at later point of time) is because this change 7d44a07b70023a3ce5286e81673d18c663c55f58 causes Element::focus() to skip "document->updateStyleIfNeeded()" if the element doesn't have a renderer. With wpe-2.28, document->updateStyleIfNeeded() was called without the renderer check.

Some logs that outline the issue

VIVEK-DBG [12503,12503]: Document::setFocusedElement (0xade103a0), old=(nil), new=0xa920d050
VIVEK-DBG [12503,12503]: AXObjectCache::handleFocusedUIElementChanged (0xad3391b8), old=(nil), new=0xa920d050
VIVEK-DBG [12503,12503]: AXObjectCache::platformHandleFocusedUIElementChanged (0xad3391b8), old=(nil), new=0xa920d050
VIVEK-DBG [12503,12503]: createFromNode, node=0xa920d050
VIVEK-DBG [12503,12503]: webkitAccessibleNew, atk_obj=0x7d72cc90, core_obj=0x959ed140
VIVEK-DBG [12503,12503]: webkitAccessibleInit, atk_obj=0x7d72cc90, accessible=0x7d72cc90, core_obj=0x959ed140
VIVEK-DBG [12503,12503]: attachWrapper, obj=0x959ed140(, Log In), wrapper=0x7d72cc90, accessible=0x7d72cc90
VIVEK-DBG [12503,12503]: attachWrapper, !document || document->childNeedsStyleRecalc()
VIVEK-DBG [12503,12503]: AXObjectCache::platformHandleFocusedUIElementChanged (0xad3391b8), sending focused event for obj=0x7d72cc90, coreobj=0x959ed140

231013-17:17:53.187 [Verbose] printEventInfo:rdkat.cpp:254 obj=0x7d72cc90, class="rdkat.Event.Object", major="state-changed", minor="focused", d1="1", d2="0", data="0"
VIVEK-DBG [12503,12503]: webkitAccessibleGetName, obj=0x7d72cc90
VIVEK-DBG [12503,12503]: AXObjectCache::get (0xad3391b8), removing node with id 118 (0xa920d050)
VIVEK-DBG [12503,12503]: detachPlatformWrapper (0x959ed140), has cache = 1
VIVEK-DBG [12503,12503]: detachWrapper, obj=0x959ed140(, Log In), wrapper=0x7d72cc90, accessible=0x7d72cc90, type=1
VIVEK-DBG [12503,12503]: webkitAccessibleDetach, accesible=0x7d72cc90, core_obj=0x959ed140
VIVEK-DBG [12503,12503]: AXObjectCache::get (0xad3391b8), removing node with id 118 (0xa920d050)
VIVEK-DBG [12503,12503]: createFromRenderer, renderer=0xa9205f40
VIVEK-DBG [12503,12503]: webkitAccessibleNew, atk_obj=0x7d7ba838, core_obj=0x959ed1e0
VIVEK-DBG [12503,12503]: webkitAccessibleInit, atk_obj=0x7d7ba838, accessible=0x7d7ba838, core_obj=0x959ed1e0
VIVEK-DBG [12503,12503]: attachWrapper, obj=0x959ed1e0(, Log In), wrapper=0x7d7ba838, accessible=0x7d7ba838

231013-17:17:53.190 [Verbose] printEventInfo:rdkat.cpp:254 obj=0x7d72cfa8, class="rdkat.Event.Object", major="children-changed", minor="add", d1="4294967295", d2="0", data="0x7d7ba838"
VIVEK-DBG [12503,12503]: webkitAccessibleGetName, accessible is detached
231013-17:17:53.190 [Verbose] printAccessibilityInfo:rdkat.cpp:269 obj=0x7d72cc90, role="invalid"

[varumugam123]

As an information, I tried two things, 1) Removed the renderer() check in Element::focus() as below, this change fixes the issue

@@ -2985,7 +2975,10 @@ void Element::focus(bool restorePreviousSelection, FocusDirection direction)
     }     RefPtr<Element> newTarget = this;

-
-    // If we don't have renderer yet, isFocusable will compute it without style update.
-    // FIXME: Expand it to avoid style update in all cases.
-    if (renderer() && document->haveStylesheetsLoaded())
+    if (document->haveStylesheetsLoaded()) 
         document->updateStyleIfNeeded();

     if (&newTarget->document() != document.ptr())
@@ -3290,7 +3283,7 @@ const RenderStyle* Element::renderOrDisplayContentsStyle() const
     return nullptr;
 } 

2) Defer handling of UI element (by the time the handler is invoked renderer is created & attached to the element). WebKitAccessible is created with a renderer.

@@ -1366,9 +1381,9 @@ void AXObjectCache::handleRowCountChanged(AXCoreObject* axObject, Document* docu void AXObjectCache::deferFocusedUIElementChangeIfNeeded(Node* oldNode, Node* newNode)
 {
-    if (nodeAndRendererAreValid(newNode) && rendererNeedsDeferredUpdate(*newNode->renderer())) {
+    if (!newNode->renderer() || (nodeAndRendererAreValid(newNode) && rendererNeedsDeferredUpdate(*newNode->renderer()))) {
         m_deferredFocusedNodeChange.append({ oldNode, newNode });
-        if (!newNode->renderer()->needsLayout() && !m_performCacheUpdateTimer.isActive())
+        if ((newNode->renderer() ? !newNode->renderer()->needsLayout() : true) && !m_performCacheUpdateTimer.isActive())
             m_performCacheUpdateTimer.startOneShot(0_s);
     } else
         handleFocusedUIElementChanged(oldNode, newNode); 

[varumugam123]

Since the issue is actually in WebKit & AXObjectCache irrespective of whether one uses atspi / atk implementation this issue should be observable

[pgorszkowski-igalia]

@varumugam123 : can you check does commit : https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/0b154529033960d5dc69d702af0b164ec008460a solve your problem?

[pgorszkowski-igalia]

@varumugam123 : did you have a chance to verify the commit which I provided?

[modeveci]

@varumugam123 @emutavchi , do we have any feedback for this proposal?

[varumugam123]

Hi @modeveci , @pgorszkowski-igalia Thanks for checking this issue, I haven't had a chance to try the commit mentioned yet.

Let me try that tomorrow and update.

[varumugam123]

@modeveci , @pgorszkowski-igalia I tried the commit, but the issue remains.

[pgorszkowski-igalia]

The fix is in upstream and waiting for review: https://github.com/WebKit/WebKit/pull/25479. After merge I will backport it to 2.38

[pgorszkowski-igalia]

@varumugam123 , @emutavchi : the PR from upstream is merged. I backported that PR to 2.38: https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/e02059bbd17bcea2b1001cfc5c72038d6d57fdcb Please verify does it solve your problem and then we can merge it with wpe-2.38 branch. Thanks

[varumugam123]

Hi @pgorszkowski-igalia, I tried the commit mentioned in the previous comment, the functionality works fine. The change fixes the issue.

[pgorszkowski-igalia]

@varumugam123 : many thanks for your verification, I will merge it in wpe-2.38.

[pgorszkowski-igalia]

It is merged also to 2.42: https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/1306