sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx | 60 ++++++++++++++++++- sc/source/ui/inc/AccessibleSpreadsheet.hxx | 2 sc/source/ui/unoobj/textuno.cxx | 10 ++- 3 files changed, 68 insertions(+), 4 deletions(-)
New commits: commit e7a0cc0f62d0647c8344d4dcbbf0325940c2ca04 Author: Patrick Luby <guibmac...@gmail.com> AuthorDate: Wed May 22 10:25:10 2024 -0400 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Mon Jun 17 15:20:31 2024 +0200 tdf#158914 add back reusing weakly cached ScAccessibleCells While commit 8e886993f32b7db11a99bdecf06451e6de6c3842 fixed tdf#157568, moving the selected cell rapidly creates a large number of stale ScAccessibleCell instances that aren't deleted until the Calc document is closed. So reduce memory usage by adding back the ScAccessibleCell cache that was in commit f22cb3dfab413a2917cd810b8e1b8f644a016327 now that a new fix for tdf#157568 has been implemented. The new fix for tdf#157568 is to do the following: - Check if the edit engine text has changed. If the input string is different than the edit engine's existing text, force update of the edit engine's text. Otherwise, the edit engine will still to be set to its existing text. - Before a cell loses focus, check if any accessible text changes have occurred and fire text and value changed notifications if needed. Change-Id: I106ad0138d5d834367be59ca625d41a692696d4a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167961 Reviewed-by: Patrick Luby <guibomac...@gmail.com> Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> (cherry picked from commit ab5ad0c8b5056da8f699cea233dd31eceb3d80a4) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168888 diff --git a/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx b/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx index 020386a69af8..52857bec9ea0 100644 --- a/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx +++ b/sc/source/ui/Accessibility/AccessibleSpreadsheet.cxx @@ -33,6 +33,7 @@ #include <com/sun/star/accessibility/AccessibleEventId.hpp> #include <com/sun/star/accessibility/AccessibleTableModelChangeType.hpp> #include <com/sun/star/lang/IndexOutOfBoundsException.hpp> +#include <comphelper/accessibletexthelper.hxx> #include <sal/log.hxx> #include <tools/gen.hxx> #include <svtools/colorcfg.hxx> @@ -315,6 +316,7 @@ void SAL_CALL ScAccessibleSpreadsheet::disposing() m_mapSelectionSend.clear(); m_mapFormulaSelectionSend.clear(); m_pAccFormulaCell.clear(); + m_mapCells.clear(); ScAccessibleTableBase::disposing(); } @@ -680,7 +682,7 @@ void ScAccessibleSpreadsheet::Notify( SfxBroadcaster& rBC, const SfxHint& rHint if (pScDoc) { OUString valStr(pScDoc->GetString(aNewCell.Col(),aNewCell.Row(),aNewCell.Tab())); - if(m_strCurCellValue != valStr) + if(mpAccCell.is() && m_strCurCellValue != valStr) { AccessibleEventObject aEvent; aEvent.EventId = AccessibleEventId::VALUE_CHANGED; @@ -755,6 +757,43 @@ void ScAccessibleSpreadsheet::CommitFocusCell(const ScAddress &aNewCell) { return ; } + + // Related tdf#158914: check if focussed cell has changed + // Some accessiblity tools, such as NVDA on Windows, appear to need + // a value changed notification before it will refetch a cell's + // accessible text. While Notify() is able to fire text and value changed + // notifications, it seems to be rarely called and when it is called, + // such as when undoing a cell, it can be called before the cell's + // accessible taxt has been updated. + // So before a cell loses focus, check if any accessible text changes + // have occurred and fire text and value changed notifications if needed. + ScDocument* pScDoc= GetDocument(mpViewShell); + if (pScDoc && mpAccCell.is()) + { + const ScAddress aOldActiveCell = mpAccCell->GetCellAddress(); + OUString valStr(pScDoc->GetString(aOldActiveCell.Col(),aOldActiveCell.Row(),aOldActiveCell.Tab())); + if(m_strCurCellValue != valStr) + { + uno::Any aOldValue; + uno::Any aNewValue; + (void)comphelper::OCommonAccessibleText::implInitTextChangedEvent(m_strCurCellValue, valStr, aOldValue, aNewValue); + AccessibleEventObject aTextChangedEvent; + aTextChangedEvent.EventId = AccessibleEventId::TEXT_CHANGED; + aTextChangedEvent.OldValue = aOldValue; + aTextChangedEvent.NewValue = aNewValue; + mpAccCell->CommitChange(aTextChangedEvent); + + if (pScDoc->HasValueData(maActiveCell)) + { + AccessibleEventObject aEvent; + aEvent.EventId = AccessibleEventId::VALUE_CHANGED; + mpAccCell->CommitChange(aEvent); + } + + m_strCurCellValue = valStr; + } + } + AccessibleEventObject aEvent; aEvent.EventId = AccessibleEventId::ACTIVE_DESCENDANT_CHANGED; aEvent.Source = uno::Reference< XAccessible >(this); @@ -763,7 +802,6 @@ void ScAccessibleSpreadsheet::CommitFocusCell(const ScAddress &aNewCell) mpAccCell = GetAccessibleCellAt(aNewCell.Row(), aNewCell.Col()); aEvent.NewValue <<= uno::Reference<XAccessible>(mpAccCell); maActiveCell = aNewCell; - ScDocument* pScDoc= GetDocument(mpViewShell); if (pScDoc) { m_strCurCellValue = pScDoc->GetString(maActiveCell.Col(),maActiveCell.Row(),maActiveCell.Tab()); @@ -931,7 +969,23 @@ rtl::Reference<ScAccessibleCell> ScAccessibleSpreadsheet::GetAccessibleCellAt(sa } else { - return ScAccessibleCell::create(this, mpViewShell, aCellAddress, getAccessibleIndex(nRow, nColumn), meSplitPos, mpAccDoc); + // tdf#158914 add back reusing weakly cached ScAccessibleCells + // While commit 8e886993f32b7db11a99bdecf06451e6de6c3842 + // fixed tdf#157568, moving the selected cell rapidly creates + // a large number of stale ScAccessibleCell instances that + // aren't deleted until the Calc document is closed. So reduce + // memory usage by adding back the ScAccessibleCell cache that + // was in commit f22cb3dfab413a2917cd810b8e1b8f644a016327 now + // that a new fix for tdf#157568 has been implemented. + 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; } } } diff --git a/sc/source/ui/inc/AccessibleSpreadsheet.hxx b/sc/source/ui/inc/AccessibleSpreadsheet.hxx index 5cf8b7018b01..49c9f1c27946 100644 --- a/sc/source/ui/inc/AccessibleSpreadsheet.hxx +++ b/sc/source/ui/inc/AccessibleSpreadsheet.hxx @@ -20,6 +20,7 @@ #pragma once #include <sal/config.h> +#include <unotools/weakref.hxx> #include <rtl/ref.hxx> @@ -267,6 +268,7 @@ 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: */ diff --git a/sc/source/ui/unoobj/textuno.cxx b/sc/source/ui/unoobj/textuno.cxx index 6102cdb9fac2..6094934a6ac6 100644 --- a/sc/source/ui/unoobj/textuno.cxx +++ b/sc/source/ui/unoobj/textuno.cxx @@ -807,7 +807,15 @@ SvxTextForwarder* ScCellTextData::GetTextForwarder() { sal_uInt32 nFormat = rDoc.GetNumberFormat(aCellPos); OUString aText = ScCellFormat::GetInputString(aCell, nFormat, *rDoc.GetFormatTable(), rDoc); - if (!aText.isEmpty()) + // tdf#157568 check if edit engine already has text + // If the input string is empty but the edit engine's existing + // text is not empty, force update of the edit engine's text. + // Otherwise, the edit engine will still to be set to its + // existing text. + // Note: CppunitTest_sc_macros_test testTdf116127 will fail if + // pEditEngine->SetTextNewDefaults() is passed an empty string + // and pEditEngine->GetText() is empty string. + if (!aText.isEmpty() || !pEditEngine->GetText().isEmpty()) pEditEngine->SetTextNewDefaults(aText, aDefaults); else pEditEngine->SetDefaults(aDefaults);