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();

Reply via email to