include/toolkit/awt/vclxaccessiblecomponent.hxx | 2 winaccessibility/inc/AccTopWindowListener.hxx | 3 winaccessibility/source/service/AccTopWindowListener.cxx | 55 ++++++++------- winaccessibility/source/service/msaaservice_impl.cxx | 16 ---- 4 files changed, 36 insertions(+), 40 deletions(-)
New commits: commit f975b8418573b887cdc3269bcefa54bde6d60fa1 Author: Michael Weghorn <[email protected]> AuthorDate: Thu Dec 5 15:22:27 2024 +0100 Commit: Michael Weghorn <[email protected]> CommitDate: Thu Dec 5 20:12:47 2024 +0100 a11y: Drop SAL_LOPLUGIN_ANNOTATE("crosscast") Added in: commit d66a97d5622bfc7c8d68452eb5a3c2378b0b1820 Date: Thu Jan 12 08:29:24 2023 +0100 loplugin:crosscast (clang-cl) > winaccessibility/source/service/AccTopWindowListener.cxx(47,36): error: suspicious dynamic cross cast from 'css::accessibility::XAccessible *' to 'VCLXAccessibleComponent *' [loplugin:crosscast] > else if (auto pvclxcomponent = dynamic_cast<VCLXAccessibleComponent*>(pAccessible)) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Change-Id: Iee7585dd2fa0d92af3be6b89d7d08326de3d1e7b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145380 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <[email protected]> AccTopWindowListener::HandleWindowOpened no longer does that crosscast now, see Change-Id: Ic96d2c95128af144c7769aac40707299b1c80f8c Author: Michael Weghorn <[email protected]> Date: Thu Dec 5 14:07:52 2024 +0000 tdf#164093 tdf#157001 wina11y: Use vcl::Window's actual XAccessible Change-Id: Ifca76a5d519099f9cbe06f8bde606e01871363fc Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177891 Reviewed-by: Michael Weghorn <[email protected]> Tested-by: Jenkins diff --git a/include/toolkit/awt/vclxaccessiblecomponent.hxx b/include/toolkit/awt/vclxaccessiblecomponent.hxx index 9227f6958035..551c7dcb607b 100644 --- a/include/toolkit/awt/vclxaccessiblecomponent.hxx +++ b/include/toolkit/awt/vclxaccessiblecomponent.hxx @@ -41,7 +41,7 @@ class AccessibleRelationSetHelper; -class TOOLKIT_DLLPUBLIC SAL_LOPLUGIN_ANNOTATE("crosscast") VCLXAccessibleComponent +class TOOLKIT_DLLPUBLIC VCLXAccessibleComponent :public cppu::ImplInheritanceHelper< comphelper::OAccessibleExtendedComponentHelper, css::lang::XServiceInfo> { commit b608604d0bbecdc24a17175374581b4e2d720658 Author: Michael Weghorn <[email protected]> AuthorDate: Thu Dec 5 14:07:52 2024 +0000 Commit: Michael Weghorn <[email protected]> CommitDate: Thu Dec 5 20:12:40 2024 +0100 tdf#164093 tdf#157001 wina11y: Use vcl::Window's actual XAccessible By default, a vcl::Window's XAccessible is its VCLXWindow (toolkit peer): css::uno::Reference< css::accessibility::XAccessible > Window::CreateAccessible() { css::uno::Reference< css::accessibility::XAccessible > xAcc( GetComponentInterface(), css::uno::UNO_QUERY ); return xAcc; } However, that's a virtual method and subclasses can return a different XAccessible. MenuFloatingWindow::CreateAccessible returns the accessible of its menu. However, winaccessibility's AccTopWindowListener code was relying on the Window's XAccessible always being its VCLXWindow. The VLCXWindow is passed as a param in the XTopWindowListener methods, and AccTopWindowListener was querying it for the XAccessible interface, and then calling XAccessible::getAccessibleContext in order to retrieve the XAccessibleContext for the window. This is incorrect if the Window actually has another XAccessible. For the df#164093 scenario with NVDA running, opening and closing the sidebar popup menu would result in the VCLXWindow's VCLXWindow::getAccessibleContext menu getting called, which calls VCLXWindow::CreateAccessibleContext and the VCLXWindow would keep a reference to the returned XAccessibleContext and dispose it when the VLCXWindow itself gets disposed. However, AccessibleFactory::createAccessibleContext (called by VCLXWindow::CreateAccessibleContext) doesn't actually create a new object for the MenuFloatingWindow's accessible, but returns the existing accessible object of the PopupMenu, which is owned by the PopupMenu, not the MenuFloatingWindow, s.a. previous commit Change-Id: Ia2931bee23204395e8b3396927acf4fa1d0f077c Author: Michael Weghorn <[email protected]> Date: Thu Dec 5 11:06:48 2024 +0000 tdf#164093 tdf#157001 a11y: Improve menu window disposal for more details. Backtrace of how that accessible context was returned: 1 `anonymous namespace'::AccessibleFactory::createAccessibleContext acc_factory.cxx 305 0x7fffbc160767 2 VCLXWindow::CreateAccessibleContext vclxwindow.cxx 879 0x7fffc0b1bc67 3 VCLXWindow::getAccessibleContext vclxwindow.cxx 2418 0x7fffc0b2670b 4 AccTopWindowListener::HandleWindowOpened AccTopWindowListener.cxx 60 0x7fffe5c32ab6 5 AccTopWindowListener::windowOpened AccTopWindowListener.cxx 119 0x7fffe5c3375e 6 ``anonymous namespace'::VCLXToolkit::callTopWindowListeners'::`2'::<lambda_1>::operator() vclxtoolkit.cxx 2364 0x7fffc0af27ef 7 comphelper::OInterfaceContainerHelper4<com::sun::star::awt::XTopWindowListener>::forEach<``anonymous namespace'::VCLXToolkit::callTopWindowListeners'::`2'::<lambda_1>> interfacecontainer4.hxx 349 0x7fffc0ae7f9e 8 `anonymous namespace'::VCLXToolkit::callTopWindowListeners vclxtoolkit.cxx 2359 0x7fffc0afeaed 9 `anonymous namespace'::VCLXToolkit::eventListenerHandler vclxtoolkit.cxx 2295 0x7fffc0b02492 10 `anonymous namespace'::VCLXToolkit::LinkStubeventListenerHandler vclxtoolkit.cxx 2288 0x7fffc0afbdf6 11 Link<VclSimpleEvent &,void>::Call link.hxx 111 0x7fffbfba33d3 12 VclEventListeners::Call vclevent.cxx 47 0x7fffbfba35f1 13 Application::ImplCallEventListeners svapp.cxx 733 0x7fffbfb76dbb 14 vcl::Window::CallEventListeners event.cxx 229 0x7fffbf45f841 15 vcl::Window::ImplSetReallyVisible window.cxx 1331 0x7fffbf566bad 16 vcl::Window::ImplSetReallyVisible window.cxx 1344 0x7fffbf566c7d 17 vcl::Window::Show window.cxx 2336 0x7fffbf56a8d1 18 vcl::Window::Show window.cxx 2349 0x7fffbf56a9ce 19 FloatingWindow::StartPopupMode floatwin.cxx 825 0x7fffbf46a702 20 PopupMenu::Run menu.cxx 3018 0x7fffbf4a6f02 ... <More> When the MenuFloatingWindow gets disposed, its VCLXWindow is also disposed. Since it incorrectly assumes it (or its Window) owns the XAccessibleContext, it disposes that as well: 1 OAccessibleMenuBaseComponent::disposing accessiblemenubasecomponent.cxx 608 0x7fffbf300c82 2 cppu::WeakComponentImplHelperBase::dispose implbase.cxx 104 0x7fffe199964a 3 cppu::PartialWeakComponentImplHelper<com::sun::star::accessibility::XAccessibleContext2,com::sun::star::accessibility::XAccessibleEventBroadcaster>::dispose compbase.hxx 90 0x7fffdb8d1820 4 VCLXWindow::dispose vclxwindow.cxx 938 0x7fffc0b24012 5 UnoWrapper::WindowDestroyed unowrapper.cxx 302 0x7fffc0cfddc6 6 vcl::Window::dispose window.cxx 220 0x7fffbf56e68f 7 SystemWindow::dispose syswin.cxx 107 0x7fffbf515606 8 FloatingWindow::dispose floatwin.cxx 225 0x7fffbf46b751 9 MenuFloatingWindow::dispose menufloatingwindow.cxx 134 0x7fffbf4bd766 10 VclReferenceBase::disposeOnce vclreferencebase.cxx 39 0x7fffbf7520d9 11 VclPtr<vcl::Window>::disposeAndClear vclptr.hxx 207 0x7fffbf3f98d2 12 PopupMenu::ImplExecute menu.cxx 2947 0x7fffbf4a0ee7 13 PopupMenu::Execute menu.cxx 2834 0x7fffbf49c392 14 MenuButton::ExecuteMenu menubtn.cxx 77 0x7fffbf639f1d 15 MenuButton::MouseButtonDown menubtn.cxx 214 0x7fffbf63a847 16 ImplHandleMouseEvent winproc.cxx 708 0x7fffbf57ca27 17 ImplHandleSalMouseButtonDown winproc.cxx 2338 0x7fffbf57e1af 18 ImplWindowFrameProc winproc.cxx 2683 0x7fffbf57fc07 19 SalFrame::CallCallback salframe.hxx 312 0x7fffbf3443c6 20 ImplHandleMouseMsg salframe.cxx 3415 0x7fffbc818e8f ... <More> However, the Menu that actually owns the accessible may still be alive, and then opening the menu again results in an attempt to make use of the already disposed object, triggering a crash due to a DisposedException being thrown: 1 RaiseException KERNELBASE 0x7ff808e8b699 2 CxxThrowException VCRUNTIME140 0x7fffef535267 3 comphelper::OCommonAccessibleComponent::ensureAlive accessiblecomponenthelper.cxx 141 0x7fffdb8d1937 4 comphelper::OContextEntryGuard::OContextEntryGuard accessiblecontexthelper.hxx 64 0x7fffbf2fb8eb 5 comphelper::OExternalLockGuard::OExternalLockGuard accessiblecontexthelper.hxx 81 0x7fffbf2fb948 6 OAccessibleMenuBaseComponent::getAccessibleContext accessiblemenubasecomponent.cxx 641 0x7fffbf301337 7 VCLXAccessibleComponent::ProcessWindowChildEvent vclxaccessiblecomponent.cxx 165 0x7fffc0a9596d 8 VCLXAccessibleComponent::WindowChildEventListener vclxaccessiblecomponent.cxx 124 0x7fffc0a96669 9 VCLXAccessibleComponent::LinkStubWindowChildEventListener vclxaccessiblecomponent.cxx 114 0x7fffc0a957f6 10 Link<VclWindowEvent &,void>::Call link.hxx 111 0x7fffbf45f7d3 11 vcl::Window::CallEventListeners event.cxx 300 0x7fffbf45fe0c 12 vcl::Window::ImplSetReallyVisible window.cxx 1331 0x7fffbf566bad 13 vcl::Window::ImplSetReallyVisible window.cxx 1344 0x7fffbf566c7d 14 vcl::Window::Show window.cxx 2336 0x7fffbf56a8d1 15 vcl::Window::Show window.cxx 2349 0x7fffbf56a9ce 16 FloatingWindow::StartPopupMode floatwin.cxx 825 0x7fffbf46a702 17 PopupMenu::Run menu.cxx 3018 0x7fffbf4a6f02 18 PopupMenu::ImplExecute menu.cxx 3005 0x7fffbf4a1707 19 PopupMenu::Execute menu.cxx 2834 0x7fffbf49c392 20 MenuButton::ExecuteMenu menubtn.cxx 77 0x7fffbf639f1d ... <More> To fix that, adjust winaccessibility's AccTopWindowListener to no longer just use the VCLXWindow for the accessible, but get the vcl::Window for the VCLXWindow and query it's actual XAccessible using vcl::Window::GetAccessible(). This fixes the tdf#164093 crash. The problem of tdf#157001 where items in the popup menu of Calc's organize template dialog are only announced the first time the menu is opened still remains for now, but is also somehow related to the MenuFloatingWindow being disposed. (No longer reproducible when no longer calling the base class FloatingWindow::dispose from MenuFloatingWindow::dispose in a local test, still needs further analysis.) Change-Id: Ic96d2c95128af144c7769aac40707299b1c80f8c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177889 Tested-by: Jenkins Reviewed-by: Michael Weghorn <[email protected]> diff --git a/winaccessibility/inc/AccTopWindowListener.hxx b/winaccessibility/inc/AccTopWindowListener.hxx index 9d8b46f95acc..2623dd2db737 100644 --- a/winaccessibility/inc/AccTopWindowListener.hxx +++ b/winaccessibility/inc/AccTopWindowListener.hxx @@ -24,6 +24,7 @@ #include <com/sun/star/accessibility/XAccessibleContext.hpp> #include <cppuhelper/implbase.hxx> +#include <vcl/window.hxx> #include "AccObjectWinManager.hxx" @@ -58,7 +59,7 @@ public: void AddAllListeners(css::accessibility::XAccessible* pAccessible, css::accessibility::XAccessible* pParentXAcc, HWND pWND); //for On-Demand load. - void HandleWindowOpened(css::accessibility::XAccessible* pAccessible); + void HandleWindowOpened(vcl::Window* pWindow); sal_Int64 GetMSComPtr(sal_Int64 hWnd, sal_Int64 lParam, sal_Int64 wParam); }; diff --git a/winaccessibility/source/service/AccTopWindowListener.cxx b/winaccessibility/source/service/AccTopWindowListener.cxx index 1b61a6436b15..f19a16b26acc 100644 --- a/winaccessibility/source/service/AccTopWindowListener.cxx +++ b/winaccessibility/source/service/AccTopWindowListener.cxx @@ -36,40 +36,38 @@ using namespace com::sun::star::uno; /** * For the new opened window, generate all the UNO accessible's object, COM object and add * accessible listener to monitor all these objects. - * @param pAccessible the accessible of the new opened window + * @param pWindow the new opened window */ -void AccTopWindowListener::HandleWindowOpened( css::accessibility::XAccessible* pAccessible ) +void AccTopWindowListener::HandleWindowOpened(vcl::Window* pWindow) { - //get SystemData from window - VclPtr<vcl::Window> window; - if (auto pvclwindow = dynamic_cast<VCLXWindow*>(pAccessible)) - window = pvclwindow->GetWindow(); - else if (auto pvclxcomponent = dynamic_cast<VCLXAccessibleComponent*>(pAccessible)) - window = pvclxcomponent->GetWindow(); - assert(window); - - const SystemEnvData* pSystemData = window->GetSystemData(); + assert(pWindow); + + const SystemEnvData* pSystemData = pWindow->GetSystemData(); if (!pSystemData) return; - Reference<css::accessibility::XAccessibleContext> xContext = pAccessible->getAccessibleContext(); + Reference<css::accessibility::XAccessible> xAccessible = pWindow->GetAccessible(); + if (!xAccessible.is()) + return; + + Reference<css::accessibility::XAccessibleContext> xContext = xAccessible->getAccessibleContext(); if(!xContext.is()) return; // add all listeners - m_aAccObjectManager.SaveTopWindowHandle(pSystemData->hWnd, pAccessible); + m_aAccObjectManager.SaveTopWindowHandle(pSystemData->hWnd, xAccessible.get()); - AddAllListeners(pAccessible, nullptr, pSystemData->hWnd); + AddAllListeners(xAccessible.get(), nullptr, pSystemData->hWnd); - if( window->GetStyle() & WB_MOVEABLE ) - m_aAccObjectManager.IncreaseState( pAccessible, static_cast<unsigned short>(-1) /* U_MOVEBLE */ ); + if (pWindow->GetStyle() & WB_MOVEABLE) + m_aAccObjectManager.IncreaseState(xAccessible.get(), static_cast<unsigned short>(-1) /* U_MOVEBLE */ ); short role = xContext->getAccessibleRole(); if (role == css::accessibility::AccessibleRole::POPUP_MENU || role == css::accessibility::AccessibleRole::MENU ) { - m_aAccObjectManager.NotifyAccEvent(pAccessible, UnoMSAAEvent::MENUPOPUPSTART); + m_aAccObjectManager.NotifyAccEvent(xAccessible.get(), UnoMSAAEvent::MENUPOPUPSTART); } if (role == css::accessibility::AccessibleRole::FRAME || @@ -77,7 +75,7 @@ void AccTopWindowListener::HandleWindowOpened( css::accessibility::XAccessible* role == css::accessibility::AccessibleRole::WINDOW || role == css::accessibility::AccessibleRole::ALERT) { - m_aAccObjectManager.NotifyAccEvent(pAccessible, UnoMSAAEvent::SHOW); + m_aAccObjectManager.NotifyAccEvent(xAccessible.get(), UnoMSAAEvent::SHOW); } } @@ -100,14 +98,15 @@ void AccTopWindowListener::windowOpened( const css::lang::EventObject& e ) if ( !e.Source.is()) return; - Reference< css::accessibility::XAccessible > xAccessible ( e.Source, UNO_QUERY ); - css::accessibility::XAccessible* pAccessible = xAccessible.get(); - if ( !pAccessible ) - return; - SolarMutexGuard g; - HandleWindowOpened( pAccessible ); + VCLXWindow* pVCLXWindow = dynamic_cast<VCLXWindow*>(e.Source.get()); + assert(pVCLXWindow && "Window is not a VCLXWindow"); + + vcl::Window* pWindow = pVCLXWindow->GetWindow(); + assert(pWindow); + + HandleWindowOpened(pWindow); } /** @@ -172,7 +171,13 @@ void AccTopWindowListener::windowClosed( const css::lang::EventObject& e ) if ( !e.Source.is()) return; - Reference< css::accessibility::XAccessible > xAccessible ( e.Source, UNO_QUERY ); + VCLXWindow* pVCLXWindow = dynamic_cast<VCLXWindow*>(e.Source.get()); + assert(pVCLXWindow && "Window is not a VCLXWindow"); + + vcl::Window* pWindow = pVCLXWindow->GetWindow(); + assert(pWindow); + + Reference<css::accessibility::XAccessible> xAccessible = pWindow->GetAccessible(); if (!xAccessible.is()) return; diff --git a/winaccessibility/source/service/msaaservice_impl.cxx b/winaccessibility/source/service/msaaservice_impl.cxx index 955d30560c6c..823238215a25 100644 --- a/winaccessibility/source/service/msaaservice_impl.cxx +++ b/winaccessibility/source/service/msaaservice_impl.cxx @@ -109,7 +109,7 @@ void MSAAServiceImpl::handleWindowOpened(sal_Int64 nAcc) if (m_pTopWindowListener.is() && nAcc) { m_pTopWindowListener->HandleWindowOpened( - static_cast<css::accessibility::XAccessible*>( + static_cast<vcl::Window*>( reinterpret_cast<void*>(nAcc))); } } @@ -142,19 +142,9 @@ Sequence< OUString > MSAAServiceImpl::getSupportedServiceNames() static void AccessBridgeHandleExistingWindow(const Reference< XMSAAService>& xAccMgr, vcl::Window* pWindow) { - assert(pWindow); - - // We have to rely on the fact that Window::GetAccessible()->getAccessibleContext() returns a valid XAccessibleContext - // also for other menus than menubar or toplevel popup window. Otherwise we had to traverse the hierarchy to find the - // context object to this menu floater. This makes the call to Window->IsMenuFloatingWindow() obsolete. - css::uno::Reference<css::accessibility::XAccessible> xAccessible = pWindow->GetAccessible(); - assert(xAccMgr.is()); - if (xAccessible.is()) - { - xAccMgr->handleWindowOpened(reinterpret_cast<sal_Int64>(xAccessible.get())); - SAL_INFO("iacc2", "Decide whether to register existing window with IAccessible2"); - } + assert(pWindow); + xAccMgr->handleWindowOpened(reinterpret_cast<sal_Int64>(pWindow)); } /*
