sc/inc/column.hxx | 1
sc/inc/table.hxx | 1
sc/qa/unit/tiledrendering/tiledrendering.cxx | 31 ++++++++++++++++++++++-----
sc/source/core/data/column.cxx | 16 +++++++++++--
sc/source/core/data/document.cxx | 10 ++++++++
sc/source/core/data/table2.cxx | 15 +++++++++++++
6 files changed, 66 insertions(+), 8 deletions(-)
New commits:
commit aa0604e81d2cafe500c3f0649c9d14d8d786ec46
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Thu Nov 30 09:13:53 2023 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Nov 30 11:38:46 2023 +0100
cppunit test for notification of note position changes on row/col changes
Change-Id: I32ed5cd249400f71903e7aa848ba63d03abbd9b2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160139
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/sc/qa/unit/tiledrendering/tiledrendering.cxx
b/sc/qa/unit/tiledrendering/tiledrendering.cxx
index b33960af5f22..27950831a1f3 100644
--- a/sc/qa/unit/tiledrendering/tiledrendering.cxx
+++ b/sc/qa/unit/tiledrendering/tiledrendering.cxx
@@ -1094,6 +1094,10 @@ CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest,
testCommentCallback)
SfxLokHelper::setView(nView1);
+ ScTabViewShell* pTabViewShell =
dynamic_cast<ScTabViewShell*>(SfxViewShell::Current());
+ if (pTabViewShell)
+ pTabViewShell->SetCursor(4, 4);
+
// Add a new comment
uno::Sequence<beans::PropertyValue>
aArgs(comphelper::InitPropertySequence(
{
@@ -1113,15 +1117,32 @@ CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest,
testCommentCallback)
CPPUNIT_ASSERT_EQUAL(std::string("LOK User1"),
aView2.m_aCommentCallbackResult.get<std::string>("author"));
CPPUNIT_ASSERT_EQUAL(std::string("Comment"),
aView1.m_aCommentCallbackResult.get<std::string>("text"));
CPPUNIT_ASSERT_EQUAL(std::string("Comment"),
aView2.m_aCommentCallbackResult.get<std::string>("text"));
- CPPUNIT_ASSERT_EQUAL(std::string("0 1 0 1"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
- CPPUNIT_ASSERT_EQUAL(std::string("0 1 0 1"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("4 4 4 4"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("4 4 4 4"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
+
+ // Ensure deleting rows updates comments
+ if (pTabViewShell)
+ pTabViewShell->SetCursor(2, 2);
+
+ dispatchCommand(mxComponent, ".uno:DeleteRows", {});
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(std::string("4 3 4 3"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("4 3 4 3"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
+
+ // Ensure deleting columns updates comments
+ if (pTabViewShell)
+ pTabViewShell->SetCursor(2, 2);
+
+ dispatchCommand(mxComponent, ".uno:DeleteColumns", {});
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(std::string("3 3 3 3"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("3 3 3 3"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
std::string aCommentId =
aView1.m_aCommentCallbackResult.get<std::string>("id");
// Edit a comment
// Select some random cell, we should be able to edit the cell note
without
// selecting the cell
- ScTabViewShell* pTabViewShell =
dynamic_cast<ScTabViewShell*>(SfxViewShell::Current());
if (pTabViewShell)
pTabViewShell->SetCursor(3, 100);
aArgs = comphelper::InitPropertySequence(
@@ -1141,8 +1162,8 @@ CPPUNIT_TEST_FIXTURE(ScTiledRenderingTest,
testCommentCallback)
CPPUNIT_ASSERT_EQUAL(std::string("LOK User2"),
aView2.m_aCommentCallbackResult.get<std::string>("author"));
CPPUNIT_ASSERT_EQUAL(std::string("Edited comment"),
aView1.m_aCommentCallbackResult.get<std::string>("text"));
CPPUNIT_ASSERT_EQUAL(std::string("Edited comment"),
aView2.m_aCommentCallbackResult.get<std::string>("text"));
- CPPUNIT_ASSERT_EQUAL(std::string("0 1 0 1"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
- CPPUNIT_ASSERT_EQUAL(std::string("0 1 0 1"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("3 3 3 3"),
aView1.m_aCommentCallbackResult.get<std::string>("cellRange"));
+ CPPUNIT_ASSERT_EQUAL(std::string("3 3 3 3"),
aView2.m_aCommentCallbackResult.get<std::string>("cellRange"));
// Delete the comment
if (pTabViewShell)
commit d9e9caf2ea2a42c09623d69c858921462aeac00e
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Wed Nov 29 20:45:33 2023 +0000
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Thu Nov 30 11:38:39 2023 +0100
No kit notification of note position changes on insert/delete row
while there is for insert/delete col.
Inserting/Deleting a column will explicitly update comments as part of a
bulk operation and block the drawing layer from updating any existing
captions before that. A side-effect of this is that the note captions
are generated for all comments in the moved cols when this happens.
While with a row the drawing layer is allowed update existing caption
positions and doesn't generate any new captions.
Presumably there's a missed optimization for insert/delete cols to
not generate extra captions that didn't exist before the change, but
leave that aside for now and add a UpdateNoteCaptions that just
notifies of note position changes when rows are inserted/deleted and
continue to piggy back on the note caption update for insert/delete
cols.
Change-Id: I4d76d15aee1de9ea5553e90b2051354bce02b1db
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160138
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index cefd03fd388e..27db1391472e 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -717,6 +717,7 @@ public:
sc::ColumnBlockPosition& rDestBlockPos, bool
bCloneCaption, SCROW nRowOffsetDest = 0) const;
void UpdateNoteCaptions( SCROW nRow1, SCROW nRow2, bool bAddressChanged =
true );
+ void CommentNotifyAddressChange( SCROW nRow1, SCROW nRow2 );
void UpdateDrawObjects( std::vector<std::vector<SdrObject*>>& pObjects,
SCROW nRowStart, SCROW nRowEnd );
void UpdateDrawObjectsForRow( std::vector<SdrObject*>& pObjects, SCCOL
nTargetCol, SCROW nTargetRow );
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 8907bde5a150..e81639ff6a53 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -522,6 +522,7 @@ public:
SCROW GetNotePosition( SCCOL nCol, size_t nIndex ) const;
void CreateAllNoteCaptions();
void ForgetNoteCaptions( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW
nRow2, bool bPreserveData );
+ void CommentNotifyAddressChange(SCCOL nCol1, SCROW nRow1, SCCOL nCol2,
SCROW nRow2);
void GetAllNoteEntries( std::vector<sc::NoteEntry>& rNotes ) const;
void GetNotesInRange( const ScRange& rRange, std::vector<sc::NoteEntry>&
rNotes ) const;
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index c448f79ae996..26fbd5cbfa5c 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -1811,11 +1811,13 @@ class NoteCaptionUpdater
{
const ScDocument& m_rDocument;
const ScAddress m_aAddress; // 'incomplete' address consisting of tab,
column
+ bool m_bUpdateCaptionPos; // false if we want to skip updating the
caption pos, only useful in kit mode
bool m_bAddressChanged; // false if the cell anchor address is unchanged
public:
- NoteCaptionUpdater(const ScDocument& rDocument, const ScAddress& rPos,
bool bAddressChanged)
+ NoteCaptionUpdater(const ScDocument& rDocument, const ScAddress& rPos,
bool bUpdateCaptionPos, bool bAddressChanged)
: m_rDocument(rDocument)
, m_aAddress(rPos)
+ , m_bUpdateCaptionPos(bUpdateCaptionPos)
, m_bAddressChanged(bAddressChanged)
{
}
@@ -1826,7 +1828,8 @@ public:
ScAddress aAddr(m_aAddress);
aAddr.SetRow(nRow);
- p->UpdateCaptionPos(aAddr);
+ if (m_bUpdateCaptionPos)
+ p->UpdateCaptionPos(aAddr);
// Notify our LOK clients
if (m_bAddressChanged)
@@ -1839,7 +1842,14 @@ public:
void ScColumn::UpdateNoteCaptions( SCROW nRow1, SCROW nRow2, bool
bAddressChanged )
{
ScAddress aAddr(nCol, 0, nTab);
- NoteCaptionUpdater aFunc(GetDoc(), aAddr, bAddressChanged);
+ NoteCaptionUpdater aFunc(GetDoc(), aAddr, true, bAddressChanged);
+ sc::ProcessNote(maCellNotes.begin(), maCellNotes, nRow1, nRow2, aFunc);
+}
+
+void ScColumn::CommentNotifyAddressChange( SCROW nRow1, SCROW nRow2 )
+{
+ ScAddress aAddr(nCol, 0, nTab);
+ NoteCaptionUpdater aFunc(GetDoc(), aAddr, false, true);
sc::ProcessNote(maCellNotes.begin(), maCellNotes, nRow1, nRow2, aFunc);
}
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 0b4ae5ca0da8..1d253db1537a 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -1304,8 +1304,13 @@ bool ScDocument::InsertRow( SCCOL nStartCol, SCTAB
nStartTab,
SetNeedsListeningGroups(aGroupPos);
for (i=nStartTab; i<=nEndTab && i < GetTableCount(); i++)
+ {
if (maTabs[i] && (!pTabMark || pTabMark->GetTableSelect(i)))
+ {
maTabs[i]->InsertRow( nStartCol, nEndCol, nStartRow, nSize );
+ maTabs[i]->CommentNotifyAddressChange(nStartCol, nStartRow,
nEndCol, MaxRow());
+ }
+ }
// UpdateRef for drawing layer must be after inserting,
// when the new row heights are known.
@@ -1425,8 +1430,13 @@ void ScDocument::DeleteRow( SCCOL nStartCol, SCTAB
nStartTab,
std::vector<ScAddress> aGroupPos;
for ( i = nStartTab; i <= nEndTab && i < GetTableCount(); i++)
+ {
if (maTabs[i] && (!pTabMark || pTabMark->GetTableSelect(i)))
+ {
maTabs[i]->DeleteRow(aCxt.maRegroupCols, nStartCol, nEndCol,
nStartRow, nSize, pUndoOutline, &aGroupPos);
+ maTabs[i]->CommentNotifyAddressChange(nStartCol, nStartRow,
nEndCol, MaxRow());
+ }
+ }
// Newly joined groups have some of their members still listening. We
// need to make sure none of them are listening.
diff --git a/sc/source/core/data/table2.cxx b/sc/source/core/data/table2.cxx
index d6c1eead2c48..52234b29b232 100644
--- a/sc/source/core/data/table2.cxx
+++ b/sc/source/core/data/table2.cxx
@@ -1941,6 +1941,21 @@ void ScTable::ForgetNoteCaptions( SCCOL nCol1, SCROW
nRow1, SCCOL nCol2, SCROW n
aCol[i].ForgetNoteCaptions(nRow1, nRow2, bPreserveData);
}
+void ScTable::CommentNotifyAddressChange( SCCOL nCol1, SCROW nRow1, SCCOL
nCol2, SCROW nRow2 )
+{
+ // Only in use in kit mode for now, but looks to me a good idea to revisit
why (since OOo times)
+ // on deleting/inserting a column that we generate all the captions, while
on deleting/inserting
+ // a row we do not. Presumably we should skip generating captions if we
don't have to.
+ if (!comphelper::LibreOfficeKit::isActive())
+ return;
+
+ if (!ValidCol(nCol1) || !ValidCol(nCol2))
+ return;
+ if ( nCol2 >= aCol.size() ) nCol2 = aCol.size() - 1;
+ for (SCCOL i = nCol1; i <= nCol2; ++i)
+ aCol[i].CommentNotifyAddressChange(nRow1, nRow2);
+}
+
void ScTable::GetAllNoteEntries( std::vector<sc::NoteEntry>& rNotes ) const
{
for (SCCOL nCol = 0; nCol < aCol.size(); ++nCol)
