sw/inc/bitmaps.hlst | 2
sw/qa/uitest/writer_tests/trackedChanges.py | 105 +++++++++++++++++++
sw/source/uibase/inc/redlndlg.hxx | 2
sw/source/uibase/misc/redlndlg.cxx | 148 +++++++++++++++++++++++++---
4 files changed, 242 insertions(+), 15 deletions(-)
New commits:
commit eebe4747d2d13545004937bb0267ccfc8ab9d63f
Author: László Németh <nem...@numbertext.org>
AuthorDate: Wed Jan 12 10:27:35 2022 +0100
Commit: László Németh <nem...@numbertext.org>
CommitDate: Wed Jan 12 14:29:28 2022 +0100
tdf#144270 sw: manage tracked table (row) deletion/insertion
in Manage Changes dialog window, where deleted/inserted
tables and table rows were shown as multiple cell changes,
as a regression. Now a single table deletion/insertion or
deleted/inserted consecutive table rows are shown with a
single tree item in the dialog window instead of showing
multiple cell changes.
Add new Action icons to the tracked table row
insertion/deletion tree items (re-using table row
deletion/insertion icons).
Show cell changes as children of the single parent
item tracked table row change.
Accept/Reject and Accept/Reject All support
1-click acceptance or rejection of table (row) changes,
instead of clicking on all cell changes of a single
table (row) deletion/insertion.
Regression from commit c4cf85766453982f1aa94a7f2cb22af19ed100be
"sw: fix crash due to redlines on tables on ooo121112-2.docx".
Change-Id: Id03a8075cc6849b70a8d32e1a955b79e7d3a3246
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128314
Tested-by: László Németh <nem...@numbertext.org>
Reviewed-by: László Németh <nem...@numbertext.org>
diff --git a/sw/inc/bitmaps.hlst b/sw/inc/bitmaps.hlst
index 93b787fafd7a..e24b79abc6cc 100644
--- a/sw/inc/bitmaps.hlst
+++ b/sw/inc/bitmaps.hlst
@@ -28,6 +28,8 @@ inline constexpr OUStringLiteral BMP_REDLINE_TABLECHG =
u"sw/res/redline_tablech
inline constexpr OUStringLiteral BMP_REDLINE_FMTCOLLSET =
u"sw/res/redline_fmtcollset.png";
inline constexpr OUStringLiteral BMP_REDLINE_MOVED_INSERTION =
u"cmd/sc_paste.png";
inline constexpr OUStringLiteral BMP_REDLINE_MOVED_DELETION =
u"cmd/sc_cut.png";
+inline constexpr OUStringLiteral BMP_REDLINE_ROW_INSERTION =
u"cmd/sc_insertrows.png";
+inline constexpr OUStringLiteral BMP_REDLINE_ROW_DELETION =
u"cmd/sc_deleterows.png";
inline constexpr OUStringLiteral RID_BMP_DB = u"sw/res/sx01.png";
inline constexpr OUStringLiteral RID_BMP_DBTABLE = u"sw/res/sx02.png";
diff --git a/sw/qa/uitest/writer_tests/trackedChanges.py
b/sw/qa/uitest/writer_tests/trackedChanges.py
index eb6d5a9f6db6..58f4957cc03d 100644
--- a/sw/qa/uitest/writer_tests/trackedChanges.py
+++ b/sw/qa/uitest/writer_tests/trackedChanges.py
@@ -217,4 +217,109 @@ class trackedchanges(UITestCase):
# Check the changes are shown after opening the Manage Tracked
Changes dialog
self.assertGreater(document.CurrentController.PageCount, 5)
+ def test_tdf144270_tracked_table_rows(self):
+ with
self.ui_test.load_file(get_url_for_data_file("TC-table-del-add.docx")) as
document:
+ xWriterDoc = self.xUITest.getTopFocusWindow()
+ xWriterEdit = xWriterDoc.getChild("writer_edit")
+
+ tables = document.getTextTables()
+ self.assertEqual(3, len(tables))
+
+ # Accept
+ with
self.ui_test.execute_modeless_dialog_through_command(".uno:AcceptTrackedChanges",
close_button="close") as xTrackDlg:
+ changesList = xTrackDlg.getChild("writerchanges")
+
+ # This was 14 (every cell is a different change instead of
counting rows or tables)
+ # Now: 4 changes (2 deleted/inserted rows and 2
deleted/inserted tables)
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ # Without the fix in place, it would have crashed here
+ for i in (3, 2, 1, 0):
+ xAccBtn = xTrackDlg.getChild("accept")
+ xAccBtn.executeAction("CLICK", tuple())
+ self.assertEqual(i, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(2, len(tables))
+
+ for i in range(1, 5):
+ xUndoBtn = xTrackDlg.getChild("undo")
+ xUndoBtn.executeAction("CLICK", tuple())
+ self.assertEqual(i, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(3, len(tables))
+
+ # Accept All
+ with
self.ui_test.execute_modeless_dialog_through_command(".uno:AcceptTrackedChanges",
close_button="close") as xTrackDlg:
+ changesList = xTrackDlg.getChild("writerchanges")
+
+ # This was 14 (every cell is a different change instead of
counting rows or tables)
+ # Now: 4 changes (2 deleted/inserted rows and 2
deleted/inserted tables)
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ xAccBtn = xTrackDlg.getChild("acceptall")
+ xAccBtn.executeAction("CLICK", tuple())
+ self.assertEqual(0, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(2, len(tables))
+
+ xUndoBtn = xTrackDlg.getChild("undo")
+ xUndoBtn.executeAction("CLICK", tuple())
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(3, len(tables))
+
+ # goto to the start of the document to reject from the first
tracked table row change
+ self.xUITest.executeCommand(".uno:GoToStartOfDoc")
+
+ # Reject
+ with
self.ui_test.execute_modeless_dialog_through_command(".uno:AcceptTrackedChanges",
close_button="close") as xTrackDlg:
+ changesList = xTrackDlg.getChild("writerchanges")
+
+ # This was 14 (every cell is a different change instead of
counting rows or tables)
+ # Now: 4 changes (2 deleted/inserted rows and 2
deleted/inserted tables)
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ # Without the fix in place, it would have crashed here
+ for i in (3, 2, 1, 0):
+ xAccBtn = xTrackDlg.getChild("reject")
+ xAccBtn.executeAction("CLICK", tuple())
+ self.assertEqual(i, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(2, len(tables))
+
+ for i in range(1, 5):
+ xUndoBtn = xTrackDlg.getChild("undo")
+ xUndoBtn.executeAction("CLICK", tuple())
+ self.assertEqual(i, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(3, len(tables))
+
+ # Reject All
+ with
self.ui_test.execute_modeless_dialog_through_command(".uno:AcceptTrackedChanges",
close_button="close") as xTrackDlg:
+ changesList = xTrackDlg.getChild("writerchanges")
+
+ # This was 14 (every cell is a different change instead of
counting rows or tables)
+ # Now: 4 changes (2 deleted/inserted rows and 2
deleted/inserted tables)
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ xAccBtn = xTrackDlg.getChild("rejectall")
+ xAccBtn.executeAction("CLICK", tuple())
+ self.assertEqual(0, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(2, len(tables))
+
+ xUndoBtn = xTrackDlg.getChild("undo")
+ xUndoBtn.executeAction("CLICK", tuple())
+ self.assertEqual(4, len(changesList.getChildren()))
+
+ tables = document.getTextTables()
+ self.assertEqual(3, len(tables))
+
# vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/sw/source/uibase/inc/redlndlg.hxx
b/sw/source/uibase/inc/redlndlg.hxx
index d488deea7fab..c91bb468a808 100644
--- a/sw/source/uibase/inc/redlndlg.hxx
+++ b/sw/source/uibase/inc/redlndlg.hxx
@@ -97,7 +97,7 @@ class SW_DLLPUBLIC SwRedlineAcceptDlg final
SAL_DLLPRIVATE void RemoveParents(SwRedlineTable::size_type
nStart, SwRedlineTable::size_type nEnd);
SAL_DLLPRIVATE void InitAuthors();
- SAL_DLLPRIVATE static OUString GetActionImage(const SwRangeRedline&
rRedln, sal_uInt16 nStack = 0);
+ SAL_DLLPRIVATE static OUString GetActionImage(const SwRangeRedline&
rRedln, sal_uInt16 nStack = 0, bool bRowChanges = false);
SAL_DLLPRIVATE OUString GetActionText(const SwRangeRedline& rRedln,
sal_uInt16 nStack = 0);
SAL_DLLPRIVATE SwRedlineTable::size_type GetRedlinePos(const
weld::TreeIter& rEntry);
diff --git a/sw/source/uibase/misc/redlndlg.cxx
b/sw/source/uibase/misc/redlndlg.cxx
index cf5041236ee7..a8099859455d 100644
--- a/sw/source/uibase/misc/redlndlg.cxx
+++ b/sw/source/uibase/misc/redlndlg.cxx
@@ -301,16 +301,20 @@ void SwRedlineAcceptDlg::InitAuthors()
m_bOnlyFormatedRedlines );
}
-OUString SwRedlineAcceptDlg::GetActionImage(const SwRangeRedline& rRedln,
sal_uInt16 nStack)
+OUString SwRedlineAcceptDlg::GetActionImage(const SwRangeRedline& rRedln,
sal_uInt16 nStack, bool bRowChanges)
{
switch (rRedln.GetType(nStack))
{
- case RedlineType::Insert: return rRedln.IsMoved()
- ? OUString(BMP_REDLINE_MOVED_INSERTION)
- : OUString(BMP_REDLINE_INSERTED);
- case RedlineType::Delete: return rRedln.IsMoved()
- ? OUString(BMP_REDLINE_MOVED_DELETION)
- : OUString(BMP_REDLINE_DELETED);
+ case RedlineType::Insert: return bRowChanges
+ ? OUString(BMP_REDLINE_ROW_INSERTION)
+ : rRedln.IsMoved()
+ ? OUString(BMP_REDLINE_MOVED_INSERTION)
+ : OUString(BMP_REDLINE_INSERTED);
+ case RedlineType::Delete: return bRowChanges
+ ? OUString(BMP_REDLINE_ROW_DELETION)
+ : rRedln.IsMoved()
+ ? OUString(BMP_REDLINE_MOVED_DELETION)
+ : OUString(BMP_REDLINE_DELETED);
case RedlineType::Format: return BMP_REDLINE_FORMATTED;
case RedlineType::ParagraphFormat: return BMP_REDLINE_FORMATTED;
case RedlineType::Table: return BMP_REDLINE_TABLECHG;
@@ -735,6 +739,30 @@ void
SwRedlineAcceptDlg::InsertParents(SwRedlineTable::size_type nStart, SwRedli
rTreeView.make_unsorted();
bool bIsShowChangesInMargin =
SW_MOD()->GetUsrPref(false)->IsShowChangesInMargin();
+
+ // collect redlines of tracked table or table row insertion/deletions
under a single tree list
+ // item to accept/reject table (row) insertion/deletion with a single
click on Accept/Reject
+ // Note: because update of the tree list depends on parent count, we
doesn't modify
+ // m_RedlineParents, only store the 2nd and more redlines as children of
the tree list
+ // item of the first redline
+
+ // count of items stored as children (to adjust parent position)
+ sal_Int32 nSkipRedlines = 0;
+ // count of items of the actual table row (or joined table rows) stored as
children =
+ // redlines of the row(s) - 1 (first redline is associated to the parent
tree list item)
+ sal_Int32 nSkipRedline = 0;
+ // nSkipRedline of the previous table row (to join multiple table rows, if
it's possible)
+ sal_Int32 nPrevSkipRedline = 0;
+
+ // last SwRangeRedline in the table row
+ SwRedlineTable::size_type nLastChangeInRow = SwRedlineTable::npos;
+ // descriptor redline of the tracked table row
+ SwRedlineTable::size_type nRowChange = SwRedlineTable::npos;
+ // descriptor redline of the previous table row to join the table rows
+ SwRedlineTable::size_type nPrevRowChange = SwRedlineTable::npos;
+
+ // show all redlines as tree list items,
+ // redlines of a tracked table (row) inserion/deletion showed as children
of a single parent
for (SwRedlineTable::size_type i = nStart; i <= nEnd; i++)
{
const SwRangeRedline& rRedln = pSh->GetRedline(i);
@@ -744,6 +772,41 @@ void
SwRedlineAcceptDlg::InsertParents(SwRedlineTable::size_type nStart, SwRedli
pRedlineParent->pData = pRedlineData;
pRedlineParent->pNext = nullptr;
+ // handle tracked table row changes
+ const SwTableBox* pTableBox;
+ const SwTableLine* pTableLine;
+ // first SwRangeRedline of the tracked table row(s), base of the
parent tree list
+ // of the other SwRangeRedlines of the tracked table row(s)
+ SwRedlineTable::size_type nNewTableParent = SwRedlineTable::npos;
+ if ( // not recognized yet as tracked table row change
+ nLastChangeInRow == SwRedlineTable::npos &&
+ nullptr != ( pTableBox =
pSh->GetRedline(i).Start()->nNode.GetNode().GetTableBox() ) &&
+ nullptr != ( pTableLine = pTableBox->GetUpper() ) &&
+ // it's a tracked row change based on the cached row data
+ RedlineType::None != pTableLine->GetRedlineType() )
+ {
+ SwRedlineTable::size_type nRedline = i;
+ nRowChange = pTableLine->UpdateTextChangesOnly(nRedline);
+ if ( SwRedlineTable::npos != nRowChange )
+ {
+ nSkipRedline = nRedline - i - 1;
+ nLastChangeInRow = nRedline - 1;
+ // join the consecutive deleted/inserted rows under a single
treebox item,
+ // if they have the same redline data (equal type, author and
time stamp)
+ if ( nPrevRowChange != SwRedlineTable::npos &&
+ pSh->GetRedline(nRowChange).GetRedlineData() ==
pSh->GetRedline(nPrevRowChange).GetRedlineData() )
+ {
+ nSkipRedline += nPrevSkipRedline + 1;
+ nPrevSkipRedline = 0;
+ nPrevRowChange = SwRedlineTable::npos;
+ }
+ else
+ nNewTableParent = i;
+ }
+ }
+
+ bool bRowChange(SwRedlineTable::npos != nLastChangeInRow);
+
bool bShowDeletedTextAsComment = bIsShowChangesInMargin &&
RedlineType::Delete == rRedln.GetType() &&
rRedln.GetComment().isEmpty();
const OUString& sComment = bShowDeletedTextAsComment
@@ -757,15 +820,28 @@ void
SwRedlineAcceptDlg::InsertParents(SwRedlineTable::size_type nStart, SwRedli
pData->pData = pRedlineParent;
pData->bDisabled = false;
- OUString sImage = GetActionImage(rRedln);
- OUString sAuthor = rRedln.GetAuthorString(0);
- pData->aDateTime = rRedln.GetTimeStamp(0);
- pData->eType = rRedln.GetType(0);
+ // use descriptor SwRangeRedline of the changed row, if needed to show
+ // the correct redline type, author and time stamp of the tracked row
change
+ const SwRangeRedline& rChangeRedln = pSh->GetRedline(bRowChange ?
nRowChange : i);
+
+ OUString sImage = GetActionImage(rChangeRedln, 0, bRowChange &&
nNewTableParent != SwRedlineTable::npos );
+ OUString sAuthor = rChangeRedln.GetAuthorString(0);
+ pData->aDateTime = rChangeRedln.GetTimeStamp(0);
+ pData->eType = rChangeRedln.GetType(0);
OUString sDateEntry = GetAppLangDateTimeString(pData->aDateTime);
OUString sId =
OUString::number(reinterpret_cast<sal_Int64>(pData.get()));
std::unique_ptr<weld::TreeIter> xParent(rTreeView.make_iterator());
- rTreeView.insert(nullptr, i, nullptr, &sId, nullptr, nullptr, false,
xParent.get());
+
+ if ( !bRowChange || nNewTableParent != SwRedlineTable::npos )
+ rTreeView.insert(nullptr, i - nSkipRedlines, nullptr, &sId,
nullptr, nullptr, false, xParent.get());
+ else
+ {
+ // put 2nd or more redlines of deleted/inserted rows as children
of their first redline
+ SwRedlineDataParent *const pParent =
m_RedlineParents[nLastChangeInRow - nSkipRedline].get();
+ rTreeView.insert(pParent->xTLBParent.get(), -1, nullptr, &sId,
nullptr, nullptr, false, xParent.get());
+ }
+
m_RedlinData.push_back(std::move(pData));
rTreeView.set_image(*xParent, sImage, -1);
@@ -785,6 +861,17 @@ void
SwRedlineAcceptDlg::InsertParents(SwRedlineTable::size_type nStart, SwRedli
pRedlineParent->xTLBParent = std::move(xParent);
InsertChildren(pRedlineParent, rRedln, bHasRedlineAutoFormat);
+
+ // end of a tracked deletion/insertion of a table row
+ if ( nLastChangeInRow != SwRedlineTable::npos && i == nLastChangeInRow
)
+ {
+ nSkipRedlines += nSkipRedline;
+ nPrevSkipRedline = nSkipRedline;
+ nSkipRedline = 0;
+ nPrevRowChange = nRowChange;
+ nNewTableParent = SwRedlineTable::npos;
+ nLastChangeInRow = SwRedlineTable::npos;
+ }
}
rTreeView.thaw();
if (m_pTable->IsSorted())
@@ -848,7 +935,21 @@ void SwRedlineAcceptDlg::CallAcceptReject( bool bSelect,
bool bAccept )
SwWait aWait( *pSh->GetView().GetDocShell(), true );
pSh->StartAction();
- if (aRedlines.size() > 1)
+ bool bMoreRedlines( aRedlines.size() > 1 ||
+ // single item with children, e.g. multiple redlines of a table or
table row deletion/insertion
+ ( aRedlines.size() == 1 && rTreeView.iter_n_children(*aRedlines[0]) >
0 ) );
+
+ // don't add extra Undo label to a single item only with redline stack
(i.e. old changes
+ // on the same text range, stored only in OOXML)
+ if ( bMoreRedlines && aRedlines.size() == 1 )
+ {
+ std::unique_ptr<weld::TreeIter> xChild(rTreeView.make_iterator(
&*aRedlines[0] ));
+ RedlinData *pData =
reinterpret_cast<RedlinData*>(rTreeView.get_id(*xChild).toInt64());
+ if ( pData->bDisabled )
+ bMoreRedlines = false;
+ }
+
+ if ( bMoreRedlines )
{
OUString aTmpStr;
{
@@ -875,9 +976,28 @@ void SwRedlineAcceptDlg::CallAcceptReject( bool bSelect,
bool bAccept )
SwRedlineTable::size_type nPosition = GetRedlinePos( *rRedLine );
if( nPosition != SwRedlineTable::npos )
(pSh->*FnAccRej)( nPosition );
+
+ // handle redlines of table rows, stored as children of the item
associated
+ // to the deleted/inserted table row(s)
+ std::unique_ptr<weld::TreeIter> xChild(rTreeView.make_iterator(
&*rRedLine ));
+ if ( rTreeView.iter_children(*xChild) )
+ {
+ RedlinData *pData =
reinterpret_cast<RedlinData*>(rTreeView.get_id(*xChild).toInt64());
+ // disabled for redline stack, but not for redlines of table rows
+ if ( !pData->bDisabled )
+ {
+ do
+ {
+ nPosition = GetRedlinePos( *xChild );
+ if( nPosition != SwRedlineTable::npos )
+ (pSh->*FnAccRej)( nPosition );
+ }
+ while ( rTreeView.iter_next_sibling(*xChild) );
+ }
+ }
}
- if (aRedlines.size() > 1)
+ if ( bMoreRedlines )
{
pSh->EndUndo();
}
