vcl/source/window/menu.cxx | 14 +----
vcl/source/window/menufloatingwindow.cxx | 5 +
winaccessibility/source/service/msaaservice_impl.cxx | 48 -------------------
3 files changed, 9 insertions(+), 58 deletions(-)
New commits:
commit f6f9a751445af8d6db7b5f497f3838fbb7efeb53
Author: Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Thu Dec 5 11:06:48 2024 +0000
Commit: Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Sat Dec 7 08:29:10 2024 +0100
tdf#164093 tdf#157001 a11y: Improve menu window disposal
In MenuFloatingWindow::dispose, unset the accessible
before calling the base class implementation, to prevent
the accessible from getting disposed there as well.
This is necessary because the MenuFloatingWindow
doesn't create (and therefore own) its accessible
object, but returns that from its menu in
MenuFloatingWindow::CreateAccessible. Therefore,
the PopupMenu as the owner is responsible for disposing the
accessible as well, not the MenuFloatingWindow.
This logic was already implemented in Menu::dispose,
but that doesn't help in cases where the MenuFloatingWindow
gets disposed independently.
In Menu::dispose, drop the extra logic
and just call `m_pWindow.disposeAndClear`
so that MenuFloatingWindow::dispose can
take care of everything that's needed.
(For the MenuBar case, MenuBar::dispose
calls MenuBar::ImplDestroy, which unsets
Menu::m_pWindow, so other than for PopupMenu,
this change to MenuBar::ImplDestroy method doesn't
make a difference for that class.)
Don't dispose the current Menu::m_pWindow in
PopupMenu::FinishRun, but instead in
PopupMenu::ImplExecute (if set) before assigning
a new one, to ensure that the old one gets disposed.
Drop the
SAL_WARN_IF(GetWindow(), "vcl", "Win?!");
because that case is handled fine now. Without this
change in place, I saw that warning getting triggered
while debugging (potentially because PopupMenu::FinishRun
never got reached as the menu didn't show, maybe due to
switching focus to the IDE when reaching a breakpoint
or different timing,...).
This commit by itself doesn't yet fix the crash
from tdf#164093 seen on Windows with NVDA running
because the menu's accessible still incorrectly gets
disposed by the MenuFloatingWindow's VCLXWindow
(toolkit window peer). That will be addressed
in a separate commit.
Change-Id: Ia2931bee23204395e8b3396927acf4fa1d0f077c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177883
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
(cherry picked from commit 6708246e20ce522e673f539369cd38687d2dd16d)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177948
diff --git a/vcl/source/window/menu.cxx b/vcl/source/window/menu.cxx
index a8c460629dd4..12c4490aa9df 100644
--- a/vcl/source/window/menu.cxx
+++ b/vcl/source/window/menu.cxx
@@ -189,15 +189,7 @@ void Menu::dispose()
{
ImplCallEventListeners( VclEventId::ObjectDying, ITEMPOS_INVALID );
- // at the window free the reference to the accessible component
- // and make sure the MenuFloatingWindow knows about our destruction
- if (m_pWindow)
- {
- MenuFloatingWindow* pFloat =
static_cast<MenuFloatingWindow*>(m_pWindow.get());
- if( pFloat->pMenu.get() == this )
- pFloat->pMenu.clear();
- m_pWindow->SetAccessible( css::uno::Reference<
css::accessibility::XAccessible >() );
- }
+ m_pWindow.disposeAndClear();
// dispose accessible components
comphelper::disposeComponent(mxAccessible);
@@ -2890,7 +2882,6 @@ sal_uInt16 PopupMenu::ImplExecute(const
VclPtr<vcl::Window>& pParentWin, const t
|
FloatWinPopupEndFlags::CloseAll);
}
- SAL_WARN_IF(GetWindow(), "vcl", "Win?!");
tools::Rectangle aRect(rRect);
aRect.SetPos(pParentWin->OutputToScreenPixel(aRect.TopLeft()));
@@ -2951,6 +2942,8 @@ sal_uInt16 PopupMenu::ImplExecute(const
VclPtr<vcl::Window>& pParentWin, const t
pWin->SetBorderStyle( WindowBorderStyle::NOBORDER );
else
pWin->SetBorderStyle( pWin->GetBorderStyle() | WindowBorderStyle::MENU
);
+
+ m_pWindow.disposeAndClear();
m_pWindow = pWin;
Size aSz = ImplCalcSize( pWin );
@@ -3084,7 +3077,6 @@ void PopupMenu::FinishRun(const
VclPtr<MenuFloatingWindow>& pWin, const VclPtr<v
pWin->StopExecute();
pWin->doShutdown();
- m_pWindow.disposeAndClear();
ImplClosePopupToolBox(pParentWin);
ImplFlushPendingSelect();
}
diff --git a/vcl/source/window/menufloatingwindow.cxx
b/vcl/source/window/menufloatingwindow.cxx
index 8beb36be0760..d49469a7ab2c 100644
--- a/vcl/source/window/menufloatingwindow.cxx
+++ b/vcl/source/window/menufloatingwindow.cxx
@@ -125,6 +125,11 @@ void MenuFloatingWindow::dispose()
pMenu.clear();
pActivePopup.clear();
xSaveFocusId.clear();
+
+ // unset accessible taken from the PopupMenu (s. CreateAccessible),
+ // it is owned and therefore disposed by the PopupMenu
+ SetAccessible(nullptr);
+
FloatingWindow::dispose();
}
commit 31323ec6f1cc06fae658fa162040575acf265c97
Author: Michael Weghorn <m.wegh...@posteo.de>
AuthorDate: Thu Dec 5 10:23:03 2024 +0000
Commit: Michael Weghorn <m.wegh...@posteo.de>
CommitDate: Sat Dec 7 08:29:01 2024 +0100
wina11y: Drop special combobox handling in AccessBridgeHandleExistingWindow
It's unclear to me why the list child's accessible should
be passed instead of the window's here.
In addition, AccessBridgeHandleExistingWindow is only relevant
when the AT brigde is started, i.e. for example when starting
a screen reader while LO has been running already. This makes
this look even less relevant and unclear to me why special handling
would be applied for that case, but not when a combobox popup
is shown when a11y has been active before already.
Change-Id: Ie7b16de36889e2432f6ac9455ab297ca16d3b26e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177882
Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
Tested-by: Jenkins
(cherry picked from commit 13a9a236adae972f360753a399bc47fda34a51d4)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/177947
diff --git a/winaccessibility/source/service/msaaservice_impl.cxx
b/winaccessibility/source/service/msaaservice_impl.cxx
index 43b0b49d8c6b..955d30560c6c 100644
--- a/winaccessibility/source/service/msaaservice_impl.cxx
+++ b/winaccessibility/source/service/msaaservice_impl.cxx
@@ -144,56 +144,10 @@ static void AccessBridgeHandleExistingWindow(const
Reference< XMSAAService>& xAc
{
assert(pWindow);
- css::uno::Reference<css::accessibility::XAccessible> xAccessible;
-
- SAL_INFO("iacc2", "Decide whether to register existing window with
IAccessible2");
-
- // Test for combo box - drop down floating windows first
- vcl::Window* pParentWindow = pWindow->GetParent();
-
- if (pParentWindow)
- {
- try
- {
- // The parent window of a combo box floating window should have
the role COMBO_BOX
- css::uno::Reference<css::accessibility::XAccessible>
xParentAccessible(pParentWindow->GetAccessible());
- if (xParentAccessible.is())
- {
- css::uno::Reference<css::accessibility::XAccessibleContext>
xParentAC(xParentAccessible->getAccessibleContext());
- if (xParentAC.is()
- && (css::accessibility::AccessibleRole::COMBO_BOX
- == xParentAC->getAccessibleRole()))
- {
- // O.k. - this is a combo box floating window
corresponding to the child of role LIST of the parent.
- // Let's not rely on a specific child order, just search
for the child with the role LIST
- sal_Int64 nCount = xParentAC->getAccessibleChildCount();
- for (sal_Int64 n = 0; (n < nCount) && !xAccessible.is();
n++)
- {
- css::uno::Reference<css::accessibility::XAccessible>
xChild = xParentAC->getAccessibleChild(n);
- if (xChild.is())
- {
-
css::uno::Reference<css::accessibility::XAccessibleContext> xChildAC =
xChild->getAccessibleContext();
- if (xChildAC.is() &&
(css::accessibility::AccessibleRole::LIST == xChildAC->getAccessibleRole()))
- {
- xAccessible = xChild;
- }
- }
- }
- }
- }
- }
- catch (css::uno::RuntimeException const&)
- {
- // Ignore show events that throw DisposedExceptions in
getAccessibleContext()
- return;
- }
- }
-
// 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.
- if (!xAccessible.is())
- xAccessible = pWindow->GetAccessible();
+ css::uno::Reference<css::accessibility::XAccessible> xAccessible =
pWindow->GetAccessible();
assert(xAccMgr.is());
if (xAccessible.is())
