summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLászló Németh <nemeth@numbertext.org>2022-01-12 10:27:35 +0100
committerXisco Fauli <xiscofauli@libreoffice.org>2022-01-12 19:08:40 +0100
commitdabf1516e83963b24cf4c54c42b02354053a2199 (patch)
tree2437a3e26f199d99d6912ce67e2c229c02382d07
parentUpdate git submodules (diff)
downloadcore-dabf1516e83963b24cf4c54c42b02354053a2199.tar.gz
core-dabf1516e83963b24cf4c54c42b02354053a2199.zip
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". (cherry-picked from commit eebe4747d2d13545004937bb0267ccfc8ab9d63f) Conflicts: sw/inc/bitmaps.hlst Change-Id: Id03a8075cc6849b70a8d32e1a955b79e7d3a3246 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/128326 Tested-by: Jenkins Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
-rw-r--r--sw/inc/bitmaps.hlst2
-rw-r--r--sw/qa/uitest/writer_tests/trackedChanges.py105
-rw-r--r--sw/source/uibase/inc/redlndlg.hxx2
-rw-r--r--sw/source/uibase/misc/redlndlg.cxx148
4 files changed, 242 insertions, 15 deletions
diff --git a/sw/inc/bitmaps.hlst b/sw/inc/bitmaps.hlst
index 2a7495d70896..2e52f1e47352 100644
--- a/sw/inc/bitmaps.hlst
+++ b/sw/inc/bitmaps.hlst
@@ -28,6 +28,8 @@
#define BMP_REDLINE_FMTCOLLSET "sw/res/redline_fmtcollset.png"
#define BMP_REDLINE_MOVED_INSERTION "cmd/sc_paste.png"
#define BMP_REDLINE_MOVED_DELETION "cmd/sc_cut.png"
+#define BMP_REDLINE_ROW_INSERTION "cmd/sc_insertrows.png"
+#define BMP_REDLINE_ROW_DELETION "cmd/sc_deleterows.png"
#define RID_BMP_COLLAPSE "res/sx18002.png"
#define RID_BMP_EXPAND "res/sx18003.png"
diff --git a/sw/qa/uitest/writer_tests/trackedChanges.py b/sw/qa/uitest/writer_tests/trackedChanges.py
index 698202bf1647..dea49c28673f 100644
--- a/sw/qa/uitest/writer_tests/trackedChanges.py
+++ b/sw/qa/uitest/writer_tests/trackedChanges.py
@@ -214,4 +214,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();
}