editeng/source/accessibility/AccessibleStaticTextBase.cxx | 15 +--
sc/CppunitTest_sc_a11y.mk | 1
sc/qa/extras/accessibility/basics.cxx | 18 ++++
sc/source/ui/Accessibility/AccessibleContextBase.cxx | 56 +++++++++-----
sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx | 10 --
sc/source/ui/inc/AccessibleContextBase.hxx | 18 +---
sc/source/ui/inc/AccessibleSpreadsheet.hxx | 3
7 files changed, 71 insertions(+), 50 deletions(-)
New commits:
commit bf421456ac92b695e8a8e40d0d434c8e52782f90
Author: Noel Grandin <noelgran...@gmail.com>
AuthorDate: Sat Oct 7 11:42:40 2023 +0200
Commit: Patrick Luby <plub...@neooffice.org>
CommitDate: Sun Oct 8 01:33:09 2023 +0200
tdf#157568 After deleting the content of a cell by pressing the delete..
.. key, Orca still speaks the content.
this reverts
commit f22cb3dfab413a2917cd810b8e1b8f644a016327
Author: Noel Grandin <noelgran...@gmail.com>
Date: Mon Jun 12 20:02:19 2023 +0200
tdf#155376 weakly cache ScAccessibleCell
which was a nice idea, but means that we would need to have
some way of updating the ScAccessibleCell when the associated
cell data changes. Which is likely to be complicated.
So return to creating new ScAccessibleCell objects all the time,
but fix them to not leak.
Change-Id: Ie17ee5c950c9809d4c7281f93761584f75256121
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/157673
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
(cherry picked from commit 8e886993f32b7db11a99bdecf06451e6de6c3842)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/157627
Reviewed-by: Michael Weghorn <m.wegh...@posteo.de>
Reviewed-by: Patrick Luby <plub...@neooffice.org>
diff --git a/editeng/source/accessibility/AccessibleStaticTextBase.cxx
b/editeng/source/accessibility/AccessibleStaticTextBase.cxx
index 954117189798..85082bc4cb21 100644
--- a/editeng/source/accessibility/AccessibleStaticTextBase.cxx
+++ b/editeng/source/accessibility/AccessibleStaticTextBase.cxx
@@ -121,8 +121,7 @@ namespace accessibility
void SetEventSource( const uno::Reference< XAccessible >& rInterface )
{
-
- mxThis = rInterface;
+ mpThis = rInterface.get();
}
void SetOffset( const Point& );
@@ -163,8 +162,8 @@ namespace accessibility
// our frontend class (the one implementing the actual
// interface). That's not necessarily the one containing the impl
- // pointer
- uno::Reference< XAccessible > mxThis;
+ // pointer. Note that this is not an uno::Reference to prevent
ref-counting cycles and leaks.
+ XAccessible* mpThis;
// implements our functionality, we're just an adapter (guarded by
solar mutex)
mutable rtl::Reference<AccessibleEditableTextPara> mxTextParagraph;
@@ -207,7 +206,7 @@ namespace accessibility
mxTextParagraph->Dispose();
// drop references
- mxThis = nullptr;
+ mpThis = nullptr;
mxTextParagraph.clear();
}
@@ -215,7 +214,7 @@ namespace accessibility
{
if( !mxTextParagraph.is() )
- throw lang::DisposedException ("object has been already disposed",
mxThis );
+ throw lang::DisposedException ("object has been already disposed",
mpThis );
// TODO: Have a different method on AccessibleEditableTextPara
// that does not care about state changes
@@ -273,7 +272,7 @@ namespace accessibility
if( nFlatIndex < 0 )
throw
lang::IndexOutOfBoundsException("AccessibleStaticTextBase_Impl::Index2Internal:
character index out of bounds",
- mxThis);
+ mpThis);
// gratuitously accepting larger indices here,
AccessibleEditableTextPara will throw eventually
sal_Int32 nCurrPara, nCurrIndex, nParas, nCurrCount;
@@ -305,7 +304,7 @@ namespace accessibility
// not found? Out of bounds
throw
lang::IndexOutOfBoundsException("AccessibleStaticTextBase_Impl::Index2Internal:
character index out of bounds",
- mxThis);
+ mpThis);
}
bool AccessibleStaticTextBase_Impl::SetSelection( sal_Int32 nStartPara,
sal_Int32 nStartIndex,
diff --git a/sc/CppunitTest_sc_a11y.mk b/sc/CppunitTest_sc_a11y.mk
index e013beb987cc..3418f2fd4c44 100644
--- a/sc/CppunitTest_sc_a11y.mk
+++ b/sc/CppunitTest_sc_a11y.mk
@@ -16,6 +16,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,sc_a11y, \
$(eval $(call gb_CppunitTest_use_libraries,sc_a11y, \
sal \
cppu \
+ cppuhelper \
subsequenttest \
test \
tl \
diff --git a/sc/qa/extras/accessibility/basics.cxx
b/sc/qa/extras/accessibility/basics.cxx
index f07a24d1a353..03b5eadabe54 100644
--- a/sc/qa/extras/accessibility/basics.cxx
+++ b/sc/qa/extras/accessibility/basics.cxx
@@ -108,5 +108,23 @@ CPPUNIT_TEST_FIXTURE(test::AccessibleTestBase,
Test64BitChildIndices)
CPPUNIT_ASSERT_EQUAL(nCol, xTable->getAccessibleColumn(nChildIndex));
}
+CPPUNIT_TEST_FIXTURE(test::AccessibleTestBase, tdf157568)
+{
+ load(u"private:factory/scalc");
+
+ uno::Reference<accessibility::XAccessibleTable> sheet(
+
getDocumentAccessibleContext()->getAccessibleChild(0)->getAccessibleContext(),
// sheet 1
+ uno::UNO_QUERY_THROW);
+
+ uno::Reference<accessibility::XAccessible> xCell =
sheet->getAccessibleCellAt(1, 1);
+ CPPUNIT_ASSERT(xCell);
+ uno::WeakReference<accessibility::XAccessible> xCellWeak(xCell);
+ xCell.clear();
+ // Verify that there are no reference cycles and that the ScAccessibleCell
object dies after we
+ // are done with it.
+ uno::Reference<accessibility::XAccessible> xCell2(xCellWeak);
+ CPPUNIT_ASSERT(!xCell2.is());
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s
cinkeys+=0=break: */
diff --git a/sc/source/ui/Accessibility/AccessibleContextBase.cxx
b/sc/source/ui/Accessibility/AccessibleContextBase.cxx
index f66f51077ca8..793669db3f26 100644
--- a/sc/source/ui/Accessibility/AccessibleContextBase.cxx
+++ b/sc/source/ui/Accessibility/AccessibleContextBase.cxx
@@ -36,6 +36,37 @@
using namespace ::com::sun::star;
using namespace ::com::sun::star::accessibility;
+/**
+ The listener is an internal class to prevent reference-counting cycles and
therefore memory leaks.
+*/
+typedef cppu::WeakComponentImplHelper<
+ css::accessibility::XAccessibleEventListener
+ > ScAccessibleContextBaseEventListenerWeakImpl;
+class ScAccessibleContextBase::ScAccessibleContextBaseEventListener : public
cppu::BaseMutex, public ScAccessibleContextBaseEventListenerWeakImpl
+{
+public:
+ ScAccessibleContextBaseEventListener(ScAccessibleContextBase& rBase)
+ : ScAccessibleContextBaseEventListenerWeakImpl(m_aMutex),
mrBase(rBase) {}
+
+ using WeakComponentImplHelperBase::disposing;
+
+ ///===== XAccessibleEventListener
========================================
+
+ virtual void SAL_CALL disposing( const lang::EventObject& rSource )
override
+ {
+ SolarMutexGuard aGuard;
+ if (rSource.Source == mrBase.mxParent)
+ dispose();
+ }
+
+ virtual void SAL_CALL
+ notifyEvent(
+ const css::accessibility::AccessibleEventObject& /*aEvent*/ ) override
{}
+private:
+ ScAccessibleContextBase& mrBase;
+};
+
+
ScAccessibleContextBase::ScAccessibleContextBase(
uno::Reference<XAccessible>
xParent,
const sal_Int16 aRole)
@@ -67,7 +98,11 @@ void ScAccessibleContextBase::Init()
{
uno::Reference< XAccessibleEventBroadcaster > xBroadcaster
(mxParent->getAccessibleContext(), uno::UNO_QUERY);
if (xBroadcaster.is())
- xBroadcaster->addAccessibleEventListener(this);
+ {
+ if (!mxEventListener)
+ mxEventListener = new
ScAccessibleContextBaseEventListener(*this);
+ xBroadcaster->addAccessibleEventListener(mxEventListener);
+ }
}
msName = createAccessibleName();
msDescription = createAccessibleDescription();
@@ -91,8 +126,8 @@ void SAL_CALL ScAccessibleContextBase::disposing()
if (mxParent.is())
{
uno::Reference< XAccessibleEventBroadcaster > xBroadcaster
(mxParent->getAccessibleContext(), uno::UNO_QUERY);
- if (xBroadcaster.is())
- xBroadcaster->removeAccessibleEventListener(this);
+ if (xBroadcaster && mxEventListener)
+ xBroadcaster->removeAccessibleEventListener(mxEventListener);
mxParent = nullptr;
}
}
@@ -378,21 +413,6 @@ void SAL_CALL
}
}
- //===== XAccessibleEventListener ========================================
-
-void SAL_CALL ScAccessibleContextBase::disposing(
- const lang::EventObject& rSource )
-{
- SolarMutexGuard aGuard;
- if (rSource.Source == mxParent)
- dispose();
-}
-
-void SAL_CALL ScAccessibleContextBase::notifyEvent(
- const AccessibleEventObject& /* aEvent */ )
-{
-}
-
// XServiceInfo
OUString SAL_CALL ScAccessibleContextBase::getImplementationName()
{
diff --git a/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx
b/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx
index 03b92c39b996..6e61edf2ddfd 100644
--- a/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx
+++ b/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx
@@ -927,15 +927,7 @@ rtl::Reference<ScAccessibleCell>
ScAccessibleSpreadsheet::GetAccessibleCellAt(sa
}
else
{
- rtl::Reference<ScAccessibleCell> xCell;
- auto it = m_mapCells.find(aCellAddress);
- if (it != m_mapCells.end())
- xCell = it->second.get();
- if (xCell)
- return xCell;
- xCell = ScAccessibleCell::create(this, mpViewShell, aCellAddress,
getAccessibleIndex(nRow, nColumn), meSplitPos, mpAccDoc);
- m_mapCells.insert(std::make_pair(aCellAddress, xCell));
- return xCell;
+ return ScAccessibleCell::create(this, mpViewShell, aCellAddress,
getAccessibleIndex(nRow, nColumn), meSplitPos, mpAccDoc);
}
}
}
diff --git a/sc/source/ui/inc/AccessibleContextBase.hxx
b/sc/source/ui/inc/AccessibleContextBase.hxx
index b7276873487f..f66dab963175 100644
--- a/sc/source/ui/inc/AccessibleContextBase.hxx
+++ b/sc/source/ui/inc/AccessibleContextBase.hxx
@@ -31,6 +31,7 @@
#include <cppuhelper/basemutex.hxx>
#include <cppuhelper/compbase.hxx>
#include <cppuhelper/implbase1.hxx>
+#include <rtl/ref.hxx>
namespace tools { class Rectangle; }
@@ -38,14 +39,12 @@ namespace tools { class Rectangle; }
This base class provides an implementation of the
<code>AccessibleContext</code> service.
*/
-
typedef cppu::WeakComponentImplHelper<
css::accessibility::XAccessible,
css::accessibility::XAccessibleComponent,
css::accessibility::XAccessibleContext,
css::accessibility::XAccessibleEventBroadcaster,
- css::lang::XServiceInfo,
- css::accessibility::XAccessibleEventListener
+ css::lang::XServiceInfo
> ScAccessibleContextBaseWeakImpl;
class ScAccessibleContextBase
@@ -53,6 +52,8 @@ class ScAccessibleContextBase
public ScAccessibleContextBaseWeakImpl,
public SfxListener
{
+class ScAccessibleContextBaseEventListener;
+
public:
//===== internal ========================================================
ScAccessibleContextBase(
@@ -160,15 +161,6 @@ public:
removeAccessibleEventListener(
const
css::uno::Reference<css::accessibility::XAccessibleEventListener>& xListener)
override;
- ///===== XAccessibleEventListener
========================================
-
- virtual void SAL_CALL
- disposing( const css::lang::EventObject& Source ) override;
-
- virtual void SAL_CALL
- notifyEvent(
- const css::accessibility::AccessibleEventObject& aEvent ) override;
-
///===== XServiceInfo
====================================================
/** Returns an identifier for the implementation of this object.
@@ -256,6 +248,8 @@ private:
/** This is the role of this object.
*/
sal_Int16 maRole;
+
+ rtl::Reference<ScAccessibleContextBaseEventListener> mxEventListener;
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/inc/AccessibleSpreadsheet.hxx
b/sc/source/ui/inc/AccessibleSpreadsheet.hxx
index cfe604c7d4d6..78e735942270 100644
--- a/sc/source/ui/inc/AccessibleSpreadsheet.hxx
+++ b/sc/source/ui/inc/AccessibleSpreadsheet.hxx
@@ -22,7 +22,6 @@
#include <sal/config.h>
#include <rtl/ref.hxx>
-#include <unotools/weakref.hxx>
#include "AccessibleTableBase.hxx"
#include "viewdata.hxx"
@@ -268,8 +267,6 @@ private:
OUString m_strCurCellValue;
ScRangeList m_LastMarkedRanges;
OUString m_strOldTabName;
- std::map<ScAddress, unotools::WeakReference< ScAccessibleCell > >
- m_mapCells;
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
