tdf#56266 sw xmloff: fix tracked deletions in insertions

RedlineSuccessorData export, i.e. tracked deletions
within tracked insertions were loaded as plain
tracked deletions during an ODF roundtrip. After that,
e.g. rejecting all changes couldn't reject the lost
tracked insertion, keeping the text of the tracked
deletion as part of the original text by mistake.

Regression from commit a9019e76812a947eb54cfa8d728c19361c929458
"INTEGRATION: CWS oasis (1.3.276); FILE MERGED
2004/05/12 11:00:37 mib 1.3.276.1: - #i20153#: changed <office:annotation> and <office:change-info>"

Note: RedlineSuccessorData is still stored in an extra
text:insertion within text:changed-region, only the
not ODF-compatible office:chg-author and office:chg-date-time
attributes were changed to dc:creator and dc:date elements
in RedlineSuccessorData export:

<text:changed-region>
<text:deletion/>
<text:insertion/>
</text:changed-region>

Because this structure still causes a bootstrap ODF validation
error in the odfexport and layout tests, check the export with
uitest. See also SetChangeInfo()/RedlineAdd().

Change-Id: Ic15c468172bd4d7ea1fd49d9b6610204f23d0036
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134860
Tested-by: Jenkins
Reviewed-by: László Németh <nemeth@numbertext.org>

3 files changed, 104 insertions, 11 deletions

diff --git a/sw/qa/uitest/data/redlinesuccessordata.docx b/sw/qa/uitest/data/redlinesuccessordata.docx
new file mode 100644
index 000000000000..c4dc0d280cdc
--- /dev/null
+++ b/sw/qa/uitest/data/redlinesuccessordata.docx
diff --git a/sw/qa/uitest/writer_tests/trackedChanges.py b/sw/qa/uitest/writer_tests/trackedChanges.py
index 0d13ddcdff8d..e6ed8c67d20b 100644
--- a/sw/qa/uitest/writer_tests/trackedChanges.py
+++ b/sw/qa/uitest/writer_tests/trackedChanges.py
@@ -9,8 +9,11 @@
 
 # tests for tracked changes ; tdf912270
 from uitest.framework import UITestCase
-from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file, type_text
+from uitest.uihelper.common import get_state_as_dict, get_url_for_data_file, type_text, select_by_text
 from libreoffice.uno.propertyvalue import mkPropertyValues
+from tempfile import TemporaryDirectory
+from org.libreoffice.unotest import systemPathToFileUrl
+import os.path
 
 class trackedchanges(UITestCase):
 
@@ -382,4 +385,87 @@ class trackedchanges(UITestCase):
                 # this was "a" (only text of the first cell was selected, not text of the row)
                 self.assertEqual(get_state_as_dict(xWriterEdit)["SelectedText"], "bca" )
 
+    def test_RedlineSuccessorData(self):
+        with TemporaryDirectory() as tempdir:
+            xFilePath = os.path.join(tempdir, "redlinesuccessordata-temp.odt")
+            with self.ui_test.load_file(get_url_for_data_file("redlinesuccessordata.docx")) as document:
+
+                # check tracked deletion in tracked insertion
+                with self.ui_test.execute_modeless_dialog_through_command('.uno:AcceptTrackedChanges', close_button="close") as xTrackDlg:
+                    changesList = xTrackDlg.getChild('writerchanges')
+                    # four children, but only three visible
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['Children'], '4')
+                    self.assertEqual(state['VisibleCount'], '3')
+
+                    # select tracked deletion with RedlineSuccessorData in tree list
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "DOWN"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['SelectEntryText'], 'Kelemen Gábor 2\t05/19/2021 12:35:00\t')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['Children'], '1')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['VisibleChildCount'], '0')
+
+                    # open tree node with the tracked insertion: four visible children
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "RIGHT"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['Children'], '4')
+                    self.assertEqual(state['VisibleCount'], '4')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['Children'], '1')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['VisibleChildCount'], '1')
+
+                    # select tracked insertion in tree list
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "DOWN"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['SelectEntryText'], 'First Person\t10/21/2012 23:45:00\t')
+
+                # Save the DOCX document as ODT with a tracked deletion in a tracked insertion
+                with self.ui_test.execute_dialog_through_command(".uno:SaveAs", close_button="open") as xSaveDialog:
+                    xFileName = xSaveDialog.getChild("file_name")
+                    xFileName.executeAction("TYPE", mkPropertyValues({"KEYCODE":"CTRL+A"}))
+                    xFileName.executeAction("TYPE", mkPropertyValues({"KEYCODE":"BACKSPACE"}))
+                    xFileName.executeAction("TYPE", mkPropertyValues({"TEXT": xFilePath}))
+                    xFileTypeCombo = xSaveDialog.getChild("file_type")
+                    select_by_text(xFileTypeCombo, "ODF Text Document (.odt)")
+
+            self.ui_test.wait_until_file_is_available(xFilePath)
+            # load the temporary file, and check ODF roundtrip of the tracked deletion in a tracked insertion
+            with self.ui_test.load_file(systemPathToFileUrl(xFilePath)) as document:
+                # check tracked deletion in tracked insertion
+                with self.ui_test.execute_modeless_dialog_through_command('.uno:AcceptTrackedChanges', close_button="close") as xTrackDlg:
+                    changesList = xTrackDlg.getChild('writerchanges')
+                    # four children, but only three visible
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['Children'], '4')
+                    self.assertEqual(state['VisibleCount'], '3')
+
+                    # select tracked deletion with RedlineSuccessorData in tree list
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "DOWN"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['SelectEntryText'], 'Kelemen Gábor 2\t05/19/2021 12:35:00\t')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['Children'], '1')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['VisibleChildCount'], '0')
+
+                    # open tree node with the tracked insertion: four visible children
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "RIGHT"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['Children'], '4')
+                    self.assertEqual(state['VisibleCount'], '4')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['Children'], '1')
+                    self.assertEqual(get_state_as_dict(changesList.getChild(1))['VisibleChildCount'], '1')
+
+                    # select tracked insertion in tree list
+                    changesList.executeAction("TYPE", mkPropertyValues({"KEYCODE": "DOWN"}))
+                    state = get_state_as_dict(changesList)
+                    self.assertEqual(state['SelectEntryText'], 'First Person\t10/21/2012 23:45:00\t')
+
+                    # reject all
+                    xAccBtn = xTrackDlg.getChild("rejectall")
+                    xAccBtn.executeAction("CLICK", tuple())
+                    # FIXME why we need double rejectall (dialog-only)?
+                    xAccBtn.executeAction("CLICK", tuple())
+                    self.assertEqual(0, len(changesList.getChildren()))
+                    # This was false, because of not rejected tracked deletion
+                    # of the text "inserts": "Document text inserts document"...
+                    self.assertTrue(document.getText().getString().startswith('Document text document text'))
+
 # vim: set shiftwidth=4 softtabstop=4 expandtab:
diff --git a/xmloff/source/text/XMLRedlineExport.cxx b/xmloff/source/text/XMLRedlineExport.cxx
index 6e5d4d79550a..eb8f8d68ad65 100644
--- a/xmloff/source/text/XMLRedlineExport.cxx
+++ b/xmloff/source/text/XMLRedlineExport.cxx
@@ -475,6 +475,7 @@ void XMLRedlineExport::ExportChangeInfo(
     WriteComment( sTmp );
 }
 
+// write RedlineSuccessorData
 void XMLRedlineExport::ExportChangeInfo(
     const Sequence<PropertyValue> & rPropertyValues)
 {
@@ -482,6 +483,9 @@ void XMLRedlineExport::ExportChangeInfo(
     bool bRemovePersonalInfo = SvtSecurityOptions::IsOptionSet(
             SvtSecurityOptions::EOption::DocWarnRemovePersonalInfo );
 
+    SvXMLElementExport aChangeInfo(rExport, XML_NAMESPACE_OFFICE,
+                                   XML_CHANGE_INFO, true, true);
+
     for(const PropertyValue& rVal : rPropertyValues)
     {
         if( rVal.Name == "RedlineAuthor" )
@@ -490,9 +494,12 @@ void XMLRedlineExport::ExportChangeInfo(
             rVal.Value >>= sTmp;
             if (!sTmp.isEmpty())
             {
-                rExport.AddAttribute(XML_NAMESPACE_OFFICE, XML_CHG_AUTHOR, bRemovePersonalInfo
-                        ? "Author" + OUString::number(rExport.GetInfoID(sTmp))
-                        : sTmp);
+                SvXMLElementExport aCreatorElem( rExport, XML_NAMESPACE_DC,
+                                        XML_CREATOR, true,
+                                        false );
+                rExport.Characters(bRemovePersonalInfo
+                                        ? "Author" + OUString::number(rExport.GetInfoID(sTmp))
+                                        : sTmp );
             }
         }
         else if( rVal.Name == "RedlineComment" )
@@ -505,9 +512,12 @@ void XMLRedlineExport::ExportChangeInfo(
             rVal.Value >>= aDateTime;
             OUStringBuffer sBuf;
             ::sax::Converter::convertDateTime(sBuf, bRemovePersonalInfo
-                    ? util::DateTime(0, 0, 0, 0, 1, 1, 1970, true) // Epoch time
-                    : aDateTime, nullptr);
-            rExport.AddAttribute(XML_NAMESPACE_OFFICE, XML_CHG_DATE_TIME, sBuf.makeStringAndClear());
+                                        ? util::DateTime(0, 0, 0, 0, 1, 1, 1970, true) // Epoch time
+                                        : aDateTime, nullptr);
+            SvXMLElementExport aDateElem( rExport, XML_NAMESPACE_DC,
+                                          XML_DATE, true,
+                                          false );
+            rExport.Characters(sBuf.makeStringAndClear());
         }
         else if( rVal.Name == "RedlineType" )
         {
@@ -520,10 +530,7 @@ void XMLRedlineExport::ExportChangeInfo(
         // else: unknown value -> ignore
     }
 
-    // finally write element
-    SvXMLElementExport aChangeInfo(rExport, XML_NAMESPACE_OFFICE,
-                                   XML_CHANGE_INFO, true, true);
-
+    // finally write comment paragraphs
     WriteComment( sComment );
 }