tdf#160806 tdf#160837 gtk3 a11y: Drop handling of some VclEventIds

Drop custom handling for VCL events `VclEventId::WindowGetFocus`
and `VclEventId::ObjectDying` in the gtk3 a11y bridge.

In my understanding, the a11y bridge should not have to
handle any of these by itself, but all relevant a11y
events should be forwarded to the a11y listener
as an appropriate `AccessibleEventObject` and be handled
by the a11y event listener instead, see
`AtkListener::notifyEvent` for the gtk3 implementation.

In a quick test with Orca, I didn't notice anything not being
announced anymore with this change in place.

If something is still not announced due to this change, a
proper fix is likely to make sure that a proper a11y event
is emitted and handled in `AtkListener::notifyEvent` instead.

Note how `VCLXAccessibleComponent::ProcessWindowEvent` already
handles the `VclEventId::WindowGetFocus` and
`VclEventId::ObjectDying` window events. This (or
a corresponding override in a subclass) and
`VclEventId::ObjectDying` are potentially good places to
start analysis if this change here results in any regressions.

Dropping this custom handling also fixes one problematic
way that an accessible object would be created for the
work window/root panel (e.g. in Writer) from the corresponding
XAccessible independent of the corresponding GTK Widget hierarchy,
thus breaking the a11y hierarchy (ascending the tree doesn't
work, see tdf#160806).

This could be observed by starting LO Writer with the gtk3 VCL
plugin first, then start Accerciser.

Printing the a11y info for the parent of the object with role
root pane as shown in Accerciser:

    In [1]: acc.parent
    Out[1]: <Atspi.Accessible object at 0x7f1f0f5b7780 (AtspiAccessible at 0x41f9c60)>
    In [2]: acc.parent.role
    Out[2]: <enum ATSPI_ROLE_FRAME of type Atspi.Role>
    In [3]: acc.parent.role
    Out[3]: <enum ATSPI_ROLE_FRAME of type Atspi.Role>

Selecting the object shown as the parent in Accerciser's tree
of LO's a11y hierarchy shows that this is not the same object:

    In [4]: acc
    Out[4]: <Atspi.Accessible object at 0x7f1f05f92100 (AtspiAccessible at 0x2d45c80)>
    In [5]: acc.role
    Out[5]: <enum ATSPI_ROLE_PANEL of type Atspi.Role>

and consequently, the first child of that accessible doesn't
report the accessible itself as its parent:

    In [6]: acc.get_child_at_index(0).parent == acc
    Out[6]: False

The commit message of

    commit 252b1591d5
    Author: Michael Weghorn <m.weghorn@posteo.de>
    Date:   Tue Apr 30 15:30:52 2024 +0200

        tdf#159369 tdf#160806 tdf#160837 gtk3 a11y: Don't skip parents one way

has some more background information.

Extra local change in place showing where the accessible object gets
created (used for analysis with rr):

    diff --git a/vcl/unx/gtk3/a11y/atkwrapper.cxx b/vcl/unx/gtk3/a11y/atkwrapper.cxx
    index db0aa1dbc907..c73d28987f26 100644
    --- a/vcl/unx/gtk3/a11y/atkwrapper.cxx
    +++ b/vcl/unx/gtk3/a11y/atkwrapper.cxx
    @@ -977,6 +977,10 @@ atk_object_wrapper_new( const css::uno::Reference< css::accessibility::XAccessib

             AtkObject* atk_obj = ATK_OBJECT(pWrap);
             atk_obj->role = mapToAtkRole(xContext->getAccessibleRole(), xContext->getAccessibleStateSet());
    +        // NOTE: the object of interest is root pane, breaks the a11y hierarchy
    +        // as it reports wrong parent
    +        if (atk_obj->role == ATK_ROLE_ROOT_PANE)
    +            SAL_WARN("vcl.gtk", "Creating wrapper for root pane");
             atk_obj->accessible_parent = parent;

             ooo_wrapper_registry_add( rxAccessible, atk_obj );

Sample backtrace of how the problematic accessible object
got created without this commit in place:

    1  atk_object_wrapper_new                                              atkwrapper.cxx       983  0x7fb5dda5ae8b
    2  atk_object_wrapper_ref                                              atkwrapper.cxx       948  0x7fb5dda5a800
    3  atk_wrapper_notify_focus_change                                     atkutil.cxx          47   0x7fb5dda4cab0
    4  handle_get_focus                                                    atkutil.cxx          451  0x7fb5dda4debd
    5  WindowEventHandler                                                  atkutil.cxx          512  0x7fb5dda4d9e3
    6  Link<VclSimpleEvent&, void>::Call                                   link.hxx             111  0x7fb5e81d4ef8
    7  VclEventListeners::Call                                             vclevent.cxx         46   0x7fb5e81d3f1a
    8  Application::ImplCallEventListeners                                 svapp.cxx            725  0x7fb5e8196b0d
    9  vcl::Window::CallEventListeners                                     event.cxx            225  0x7fb5e779f6ee
    10 vcl::Window::PreNotify                                              event.cxx            70   0x7fb5e779f58c
    11 SystemWindow::PreNotify                                             syswin.cxx           245  0x7fb5e788e470
    12 vcl::Window::CompatPreNotify                                        window.cxx           3941 0x7fb5e790cddd
    13 ImplCallPreNotify                                                   winproc.cxx          69   0x7fb5e791d061
    14 vcl::Window::ImplGrabFocus                                          mouse.cxx            383  0x7fb5e782ffc5
    15 vcl::Window::GrabFocus                                              window.cxx           2988 0x7fb5e78fa027
    16 vcl::Window::ImplAsyncFocusHdl                                      winproc.cxx          2061 0x7fb5e7923386
    17 vcl::Window::LinkStubImplAsyncFocusHdl                              winproc.cxx          2035 0x7fb5e792318d
    18 Link<void *, void>::Call                                            link.hxx             111  0x7fb5e792c298
    19 ImplHandleUserEvent                                                 winproc.cxx          2287 0x7fb5e7927f79
    20 ImplWindowFrameProc                                                 winproc.cxx          2851 0x7fb5e7924930
    21 SalFrame::CallCallback                                              salframe.hxx         312  0x7fb5e85a0310
    22 SalGenericDisplay::ProcessEvent                                     gendisp.cxx          66   0x7fb5e85cae6f
    23 SalUserEventList::DispatchUserEvents(bool)::$_0::operator()() const salusereventlist.cxx 119  0x7fb5e80b49bd
    24 SalUserEventList::DispatchUserEvents                                salusereventlist.cxx 120  0x7fb5e80b4864
    25 SalGenericDisplay::DispatchInternalEvent                            gendisp.cxx          51   0x7fb5e85cadc5
    26 call_userEventFn                                                    gtkdata.cxx          824  0x7fb5dda8e216
    27 ??                                                                                            0x7fb5e47110d9
    28 ??                                                                                            0x7fb5e4714317
    29 g_main_context_iteration                                                                      0x7fb5e4714930
    30 GtkSalData::Yield                                                   gtkdata.cxx          405  0x7fb5dda8ccef
    31 GtkInstance::DoYield                                                gtkinst.cxx          435  0x7fb5dda92173
    32 ImplYield                                                           svapp.cxx            378  0x7fb5e8194dfc
    33 Application::Yield                                                  svapp.cxx            466  0x7fb5e819470b
    34 Application::Execute                                                svapp.cxx            353  0x7fb5e81944a2
    35 desktop::Desktop::Main                                              app.cxx              1615 0x7fb5f13254fc
    36 ImplSVMain                                                          svmain.cxx           229  0x7fb5e81b760e
    37 SVMain                                                              svmain.cxx           261  0x7fb5e81b9659
    38 soffice_main                                                        sofficemain.cxx      93   0x7fb5f13a7eb3
    39 sal_main                                                            main.c               51   0x55b83ad6da5d
    40 main                                                                main.c               49   0x55b83ad6da37

Sample backtrace for how the root pane a11y object is created for the
good case, i.e. the case where the root pane's parent is set properly
as it uses the GTK widget hierarchy (backtrace obtained from an rr run
when first starting Accerciser, then LO, with master as of
7a895ec420 + the same local change as mentioned
above):

    1  atk_object_wrapper_new            atkwrapper.cxx  983  0x7f3799a5b48b
    2  wrapper_factory_create_accessible atkfactory.cxx  83   0x7f3799a2c9c1
    3  ooo_fixed_get_accessible          atkfactory.cxx  95   0x7f3799a2c6d2
    4  ??                                                     0x7f37993ec4c8
    5  ??                                                     0x7f3798c536d3
    6  ??                                                     0x7f379f9110d9
    7  ??                                                     0x7f379f914317
    8  g_main_context_iteration                               0x7f379f914930
    9  GtkSalData::Yield                 gtkdata.cxx     405  0x7f3799a8d2df
    10 GtkInstance::DoYield              gtkinst.cxx     435  0x7f3799a92743
    11 ImplYield                         svapp.cxx       378  0x7f37a4194dfc
    12 Application::Yield                svapp.cxx       466  0x7f37a419470b
    13 Application::Execute              svapp.cxx       353  0x7f37a41944a2
    14 desktop::Desktop::Main            app.cxx         1615 0x7f37ad3254fc
    15 ImplSVMain                        svmain.cxx      229  0x7f37a41b760e
    16 SVMain                            svmain.cxx      261  0x7f37a41b9659
    17 soffice_main                      sofficemain.cxx 93   0x7f37ad3a7eb3
    18 sal_main                          main.c          51   0x560d4a1f4a5d
    19 main                              main.c          49   0x560d4a1f4a37

Similar changes for other events will follow.
(Keeping them separate for easier bibisecting/analysis if any
of those changes causes regressions.)

Change-Id: I00e62016ced2fbb8796960671f5e58a3ceac4b29
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167208
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This commit is contained in:
Michael Weghorn 2024-05-06 14:44:36 +02:00
parent 1b9041a7c9
commit c45c64aeb5

View file

@ -378,22 +378,6 @@ static void handle_toolbox_buttonchange(VclWindowEvent const *pEvent)
}
}
/*****************************************************************************/
namespace {
struct WindowList {
~WindowList() { assert(list.empty()); };
// needs to be empty already on DeInitVCL, but at least check it's empty
// on exit
std::set< VclPtr<vcl::Window> > list;
};
WindowList g_aWindowList;
}
rtl::Reference<DocumentFocusListener> GtkSalData::GetDocumentFocusListener()
{
rtl::Reference<DocumentFocusListener> xDFL = m_xDocumentFocusListener.get();
@ -405,69 +389,6 @@ rtl::Reference<DocumentFocusListener> GtkSalData::GetDocumentFocusListener()
return xDFL;
}
static void handle_get_focus(::VclWindowEvent const * pEvent)
{
GtkSalData *const pSalData(GetGtkSalData());
assert(pSalData);
rtl::Reference<DocumentFocusListener> xDocumentFocusListener(pSalData->GetDocumentFocusListener());
vcl::Window *pWindow = pEvent->GetWindow();
// The menu bar is handled through VclEventId::MenuHighlightED
if( ! pWindow || !pWindow->IsReallyVisible() || pWindow->GetType() == WindowType::MENUBARWINDOW )
return;
// ToolBoxes are handled through VclEventId::ToolboxHighlight
if( pWindow->GetType() == WindowType::TOOLBOX )
return;
if( pWindow->GetType() == WindowType::TABCONTROL )
{
handle_tabpage_activated( pWindow );
return;
}
uno::Reference< accessibility::XAccessible > xAccessible =
pWindow->GetAccessible();
if( ! xAccessible.is() )
return;
uno::Reference< accessibility::XAccessibleContext > xContext =
xAccessible->getAccessibleContext();
if( ! xContext.is() )
return;
sal_Int64 nStateSet = xContext->getAccessibleStateSet();
/* the UNO ToolBox wrapper does not (yet?) support XAccessibleSelection, so we
* need to add listeners to the children instead of re-using the tabpage stuff
*/
if( (nStateSet & accessibility::AccessibleStateType::FOCUSED) &&
( pWindow->GetType() != WindowType::TREELISTBOX ) )
{
atk_wrapper_notify_focus_change(xAccessible);
}
else
{
if( g_aWindowList.list.insert(pWindow).second )
{
try
{
xDocumentFocusListener->attachRecursive(xAccessible, xContext, nStateSet);
}
catch (const uno::Exception&)
{
g_warning( "Exception caught processing focus events" );
}
}
}
}
/*****************************************************************************/
static void handle_menu_highlighted(::VclMenuEvent const * pEvent)
{
try
@ -502,27 +423,6 @@ static void WindowEventHandler(void *, VclSimpleEvent& rEvent)
{
switch (rEvent.GetId())
{
case VclEventId::WindowShow:
break;
case VclEventId::WindowHide:
break;
case VclEventId::WindowClose:
break;
case VclEventId::WindowGetFocus:
handle_get_focus(static_cast< ::VclWindowEvent const * >(&rEvent));
break;
case VclEventId::WindowLoseFocus:
break;
case VclEventId::WindowMinimize:
break;
case VclEventId::WindowNormalize:
break;
case VclEventId::WindowKeyInput:
case VclEventId::WindowKeyUp:
case VclEventId::WindowCommand:
case VclEventId::WindowMouseMove:
break;
case VclEventId::MenuHighlight:
if (const VclMenuEvent* pMenuEvent = dynamic_cast<const VclMenuEvent*>(&rEvent))
{
@ -538,9 +438,6 @@ static void WindowEventHandler(void *, VclSimpleEvent& rEvent)
handle_toolbox_buttonchange(static_cast< ::VclWindowEvent const * >(&rEvent));
break;
case VclEventId::ObjectDying:
g_aWindowList.list.erase( static_cast< ::VclWindowEvent const * >(&rEvent)->GetWindow() );
[[fallthrough]];
case VclEventId::ToolboxHighlightOff:
handle_toolbox_highlightoff(static_cast< ::VclWindowEvent const * >(&rEvent)->GetWindow());
break;