mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tdf#144270 sw: manage tracked table (row) deletion/insertion

Browse Source 
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 c4cf857664
"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 <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>
This commit is contained in:
László Németh
2022-01-12 10:27:35 +01:00
parent 439df74885
commit eebe4747d2
4 changed files with 242 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
sw/inc/bitmaps.hlst
View File

@@ -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";

105
sw/qa/uitest/writer_tests/trackedChanges.py
View File

@@ -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:

2
sw/source/uibase/inc/redlndlg.hxx
View File

@@ -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);

148
sw/source/uibase/misc/redlndlg.cxx
View File

@@ -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();
}