accessibility/inc/standard/vclxaccessiblelist.hxx | 2 accessibility/source/standard/vclxaccessiblebox.cxx | 57 +++++++------------ accessibility/source/standard/vclxaccessiblelist.cxx | 10 ++- 3 files changed, 31 insertions(+), 38 deletions(-)
New commits: commit 8f88aff98c6089140c73437e567e6d78da5f563e Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Tue Feb 27 21:42:42 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Wed Feb 28 09:02:46 2024 +0100 tdf#159910 a11y: Dispose VCLXAccessibleList children In `VCLXAccessibleList::HandleChangedItemList`, don't just clear the vector of list items, but dispose them first. To avoid code duplication, extract a helper method from `VCLXAccessibleList::disposing` which already disposes the children since commit 51de048ae97cbd371457dbc07120e30db9ee4187 Author: Michael Weghorn <m.wegh...@posteo.de> Date: Mon Sep 4 17:19:03 2023 +0200 tdf#157088 a11y: Dispose list items with list This fixes a similar crash/assert on exit, seen after using the Navigator in Writer (in particular the combo/listboxes in there) with the qt6 VCL plugin and the Orca screen reader running. stderr showed this: soffice.bin: .../libreoffice/comphelper/source/misc/accessibleeventnotifier.cxx:142: bool (anonymous namespace)::implLookupClient(const AccessibleEventNotifier::TClientId, ClientMap::iterator &): Assertion `rClients.end() != rPos && "AccessibleEventNotifier::implLookupClient: invalid client id " "(did you register your client?)!"' failed. Aborted Backtrace: 1 __pthread_kill_implementation pthread_kill.c 44 0x7f0e1a4a816c 2 __pthread_kill_internal pthread_kill.c 78 0x7f0e1a4a81cf 3 __GI_raise raise.c 26 0x7f0e1a45a472 4 __GI_abort abort.c 79 0x7f0e1a4444b2 5 __assert_fail_base assert.c 92 0x7f0e1a4443d5 6 __assert_fail assert.c 101 0x7f0e1a4533a2 7 (anonymous namespace)::implLookupClient accessibleeventnotifier.cxx 140 0x7f0e18ce59ac 8 comphelper::AccessibleEventNotifier::revokeClientNotifyDisposing accessibleeventnotifier.cxx 185 0x7f0e18ce5e68 9 VCLXAccessibleListItem::disposing vclxaccessiblelistitem.cxx 164 0x7f0ddf7d237f 10 cppu::WeakComponentImplHelperBase::dispose implbase.cxx 104 0x7f0e1873f544 11 cppu::PartialWeakComponentImplHelper<com::sun::star::accessibility::XAccessible, com::sun::star::accessibility::XAccessibleContext, com::sun::star::accessibility::XAccessibleComponent, com::sun::star::accessibility::XAccessibleEventBroadcaster, com::sun::star::accessibility::XAccessibleText, com::sun::star::lang::XServiceInfo>::dispose compbase.hxx 90 0x7f0ddf7cb7c5 12 cppu::WeakComponentImplHelperBase::release implbase.cxx 79 0x7f0e1873f1fe 13 cppu::PartialWeakComponentImplHelper<com::sun::star::accessibility::XAccessible, com::sun::star::accessibility::XAccessibleContext, com::sun::star::accessibility::XAccessibleComponent, com::sun::star::accessibility::XAccessibleEventBroadcaster, com::sun::star::accessibility::XAccessibleText, com::sun::star::lang::XServiceInfo>::release compbase.hxx 86 0x7f0ddf7cd0c5 14 com::sun::star::uno::Reference<com::sun::star::accessibility::XAccessible>::~Reference Reference.hxx 114 0x7f0e06bbccbe 15 QtAccessibleWidget::~QtAccessibleWidget QtAccessibleWidget.hxx 39 0x7f0e06bd618d 16 QtAccessibleWidget::~QtAccessibleWidget QtAccessibleWidget.hxx 39 0x7f0e06bd6219 17 QAccessibleCache::deleteInterface qaccessiblecache.cpp 173 0x7f0e05545319 18 QAccessibleCache::~QAccessibleCache qaccessiblecache.cpp 31 0x7f0e0554492a 19 QAccessibleCache::~QAccessibleCache qaccessiblecache.cpp 32 0x7f0e055449b0 20 cleanupAccessibleCache qaccessiblecache.cpp 24 0x7f0e05544896 21 qt_call_post_routines qcoreapplication.cpp 332 0x7f0e05faf826 22 QApplication::~QApplication qapplication.cpp 665 0x7f0e0419e24a 23 QApplication::~QApplication qapplication.cpp 722 0x7f0e0419e55c 24 std::default_delete<QApplication>::operator() unique_ptr.h 99 0x7f0e06c5d63c 25 std::__uniq_ptr_impl<QApplication, std::default_delete<QApplication>>::reset unique_ptr.h 211 0x7f0e06c5df7c 26 std::unique_ptr<QApplication, std::default_delete<QApplication>>::reset unique_ptr.h 509 0x7f0e06c58bfd 27 QtInstance::~QtInstance QtInstance.cxx 305 0x7f0e06c51184 28 QtInstance::~QtInstance QtInstance.cxx 302 0x7f0e06c51279 29 DestroySalInstance salplug.cxx 368 0x7f0e114c0a18 30 DeInitVCL svmain.cxx 625 0x7f0e115c5e7d 31 ImplSVMain svmain.cxx 254 0x7f0e115c4031 32 SVMain svmain.cxx 261 0x7f0e115c5f79 33 soffice_main sofficemain.cxx 94 0x7f0e1a7a4ba3 34 sal_main main.c 51 0x559ce9c67a5d 35 main main.c 49 0x559ce9c67a37 Change-Id: Ic5121645a6920a8ac35154dda1dcfa1974ab9d4a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164062 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> diff --git a/accessibility/inc/standard/vclxaccessiblelist.hxx b/accessibility/inc/standard/vclxaccessiblelist.hxx index f668e75d7cda..7bfb83c7e386 100644 --- a/accessibility/inc/standard/vclxaccessiblelist.hxx +++ b/accessibility/inc/standard/vclxaccessiblelist.hxx @@ -136,6 +136,8 @@ private: */ virtual void SAL_CALL disposing() override; + void disposeChildren(); + /** This method adds the states AccessibleStateType::FOCUSABLE and possibly AccessibleStateType::MULTI_SELECTABLE to the state set diff --git a/accessibility/source/standard/vclxaccessiblelist.cxx b/accessibility/source/standard/vclxaccessiblelist.cxx index 36573dcbb385..00a4258f602e 100644 --- a/accessibility/source/standard/vclxaccessiblelist.cxx +++ b/accessibility/source/standard/vclxaccessiblelist.cxx @@ -108,6 +108,12 @@ void SAL_CALL VCLXAccessibleList::disposing() { VCLXAccessibleComponent::disposing(); + disposeChildren(); + m_pListBoxHelper.reset(); +} + +void VCLXAccessibleList::disposeChildren() +{ // Dispose all items in the list. for (rtl::Reference<VCLXAccessibleListItem>& rxChild : m_aAccessibleChildren) { @@ -116,8 +122,6 @@ void SAL_CALL VCLXAccessibleList::disposing() } m_aAccessibleChildren.clear(); - - m_pListBoxHelper.reset(); } @@ -515,7 +519,7 @@ rtl::Reference<VCLXAccessibleListItem> VCLXAccessibleList::CreateChild(sal_Int32 void VCLXAccessibleList::HandleChangedItemList() { - m_aAccessibleChildren.clear(); + disposeChildren(); NotifyAccessibleEvent ( AccessibleEventId::INVALIDATE_ALL_CHILDREN, Any(), Any()); commit 0845e782eb271c6e13ad376cff7ec72724d2a89c Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Tue Feb 27 20:51:41 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Wed Feb 28 09:02:40 2024 +0100 a11y: Drop extra local variable Just assign to `VCLXAccessibleBox::m_xList` right away. Change-Id: Id6cd871ccb54cd709ddf3ed6e59b8f3feb8314ed Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164061 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> diff --git a/accessibility/source/standard/vclxaccessiblebox.cxx b/accessibility/source/standard/vclxaccessiblebox.cxx index 4562a0fc40c0..43b9967ed51a 100644 --- a/accessibility/source/standard/vclxaccessiblebox.cxx +++ b/accessibility/source/standard/vclxaccessiblebox.cxx @@ -293,11 +293,10 @@ Reference<XAccessible> SAL_CALL VCLXAccessibleBox::getAccessibleChild (sal_Int64 // List. if ( ! m_xList.is()) { - rtl::Reference<VCLXAccessibleList> pList = new VCLXAccessibleList ( GetVCLXWindow(), + m_xList = new VCLXAccessibleList(GetVCLXWindow(), (m_aBoxType == LISTBOX ? VCLXAccessibleList::LISTBOX : VCLXAccessibleList::COMBOBOX), this); - pList->SetIndexInParent (i); - m_xList = pList; + m_xList->SetIndexInParent(i); } xChild = m_xList; } commit dd514841e6d9b1b2f9d1ac13bca593fcfcb781f6 Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Tue Feb 27 20:34:59 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Wed Feb 28 09:02:34 2024 +0100 a11y: Just use reference instead of extra pointer to content Just use the `VCLXAccessibleBox::m_xList` member directly instead of extra local pointer variables referring to the reference's body. Change-Id: Ic760d766221df773e8285c927e58ea7419cc7da6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164060 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> diff --git a/accessibility/source/standard/vclxaccessiblebox.cxx b/accessibility/source/standard/vclxaccessiblebox.cxx index 93e49f90c285..4562a0fc40c0 100644 --- a/accessibility/source/standard/vclxaccessiblebox.cxx +++ b/accessibility/source/standard/vclxaccessiblebox.cxx @@ -108,15 +108,13 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven case VclEventId::ListboxSelect: { // Forward the call to the list child. - VCLXAccessibleList* pList = m_xList.get(); - if ( pList == nullptr ) + if (!m_xList.is()) { getAccessibleChild ( m_bHasTextChild ? 1 : 0 ); - pList = m_xList.get(); } - if ( pList != nullptr ) + if (m_xList.is()) { - pList->ProcessWindowEvent (rVclWindowEvent, m_bIsDropDownBox); + m_xList->ProcessWindowEvent(rVclWindowEvent, m_bIsDropDownBox); #if defined(_WIN32) if (m_bIsDropDownBox) { @@ -128,30 +126,26 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven } case VclEventId::DropdownOpen: { - VCLXAccessibleList* pList = m_xList.get(); - if ( pList == nullptr ) + if (!m_xList.is()) { getAccessibleChild ( m_bHasTextChild ? 1 : 0 ); - pList = m_xList.get(); } - if ( pList != nullptr ) + if (m_xList.is()) { - pList->ProcessWindowEvent (rVclWindowEvent); - pList->HandleDropOpen(); + m_xList->ProcessWindowEvent(rVclWindowEvent); + m_xList->HandleDropOpen(); } break; } case VclEventId::DropdownClose: { - VCLXAccessibleList* pList = m_xList.get(); - if ( pList == nullptr ) + if (!m_xList.is()) { getAccessibleChild ( m_bHasTextChild ? 1 : 0 ); - pList = m_xList.get(); } - if ( pList != nullptr ) + if (m_xList.is()) { - pList->ProcessWindowEvent (rVclWindowEvent); + m_xList->ProcessWindowEvent(rVclWindowEvent); } VclPtr<vcl::Window> pWindow = GetWindow(); if( pWindow && (pWindow->HasFocus() || pWindow->HasChildPathFocus()) ) @@ -164,8 +158,7 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven } case VclEventId::ComboboxSelect: { - VCLXAccessibleList* pList = m_xList.get(); - if (pList != nullptr && m_xText.is()) + if (m_xList.is() && m_xText.is()) { Reference<XAccessibleText> xText (m_xText->getAccessibleContext(), UNO_QUERY); if ( xText.is() ) @@ -173,7 +166,7 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven OUString sText = xText->getSelectedText(); if ( sText.isEmpty() ) sText = xText->getText(); - pList->UpdateSelection_Acc(sText, m_bIsDropDownBox); + m_xList->UpdateSelection_Acc(sText, m_bIsDropDownBox); #if defined(_WIN32) if (m_bIsDropDownBox || m_aBoxType==COMBOBOX) NotifyAccessibleEvent(AccessibleEventId::VALUE_CHANGED, Any(), Any()); @@ -193,14 +186,12 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven case VclEventId::ComboboxItemRemoved: { // Forward the call to the list child. - VCLXAccessibleList* pList = m_xList.get(); - if ( pList == nullptr ) + if (!m_xList.is()) { getAccessibleChild ( m_bHasTextChild ? 1 : 0 ); - pList = m_xList.get(); } - if ( pList != nullptr ) - pList->ProcessWindowEvent (rVclWindowEvent); + if (m_xList.is()) + m_xList->ProcessWindowEvent(rVclWindowEvent); break; } @@ -211,8 +202,7 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven // the same VCL object as this box does. In case of the // combobox, however, we have to help by providing the list with // the text of the currently selected item. - VCLXAccessibleList* pList = m_xList.get(); - if (pList != nullptr && m_xText.is()) + if (m_xList.is() && m_xText.is()) { Reference<XAccessibleText> xText (m_xText->getAccessibleContext(), UNO_QUERY); if ( xText.is() ) @@ -220,7 +210,7 @@ void VCLXAccessibleBox::ProcessWindowEvent (const VclWindowEvent& rVclWindowEven OUString sText = xText->getSelectedText(); if ( sText.isEmpty() ) sText = xText->getText(); - pList->UpdateSelection (sText); + m_xList->UpdateSelection(sText); } } break; @@ -443,13 +433,11 @@ Any VCLXAccessibleBox::getCurrentValue( ) } if (m_aBoxType == LISTBOX && m_bIsDropDownBox && m_xList.is() ) { - - VCLXAccessibleList* pList = m_xList.get(); - if(pList->IsInDropDown()) + if (m_xList->IsInDropDown()) { - if(pList->getSelectedAccessibleChildCount()>0) + if (m_xList->getSelectedAccessibleChildCount() > 0) { - Reference<XAccessibleContext> xName (pList->getSelectedAccessibleChild(sal_Int64(0)), UNO_QUERY); + Reference<XAccessibleContext> xName (m_xList->getSelectedAccessibleChild(sal_Int64(0)), UNO_QUERY); if(xName.is()) { aAny <<= xName->getAccessibleName();