schema/libreoffice/OpenDocument-v1.4+libreoffice-schema.rng | 11 +++
sw/source/core/doc/docredln.cxx | 22 +++++++
sw/source/core/unocore/unoredline.cxx | 21 ++++++-
xmloff/qa/unit/data/del-then-format.docx |binary
xmloff/qa/unit/text.cxx | 21 +++++++
xmloff/source/text/XMLRedlineExport.cxx | 34 +++++++++---
6 files changed, 97 insertions(+), 12 deletions(-)
New commits:
commit 80139ffdd67f53535eaf784cdc2324b5fae9969d
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Tue May 20 08:25:44 2025 +0200
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Tue May 20 09:35:41 2025 +0200
tdf#166319 sw interdependent redlines: handle ODF export of delete under
format
The bugdoc has <del>AA<format>BB</format>CC</del> in it, DOCX import
works, but exporting to ODT turns the format-on-delete redline into a
plain "format" redline.
What happens is that various code only checks the m_eType of the first
SwRedlineData of an SwRangeRedline, so format-on-delete is handled as
format. But this is not correct: if the format-on-delete is to be
accepted, then it's just a small detail that there was a format on top
of it: the result should be text deleted from the document on accept,
not accepting the the format changes and leaving the text in the
document.
Fix the problem by:
1) Extending SwRangeRedline::Hide() and SwRangeRedline::Show(), so when
the ODF export moves delete redlines to their own toplevel SwNode
section, so it can refer to these ranges using a single SwStartNode
index, this move happens for format-on-delete as well, not just deletes.
2) Then can extend SwXRedlinePortion::GetPropertyValue()'s
lcl_GetSuccessorProperties() to produce an XText for the deleted
range of a format-on-delete redline.
3) Finally consume this new XText in
XMLRedlineExport::ExportChangeInfo().
Note that this way a format redline can now have its own XText (in case
it's on top of a delete), but we want to export the text only once, so
restrict the content export for a redline to the 'delete' type.
Change-Id: I91902a2ae4b464ace41717a196d19530be9d84d1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185548
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
diff --git a/schema/libreoffice/OpenDocument-v1.4+libreoffice-schema.rng
b/schema/libreoffice/OpenDocument-v1.4+libreoffice-schema.rng
index 6a8e3149c66e..10023519509e 100644
--- a/schema/libreoffice/OpenDocument-v1.4+libreoffice-schema.rng
+++ b/schema/libreoffice/OpenDocument-v1.4+libreoffice-schema.rng
@@ -4034,4 +4034,15 @@
xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.
</rng:attribute>
</rng:optional>
</rng:define>
+
+ <!-- TODO(vmiklos) no proposal for insert/delete redline under another
redline -->
+ <rng:define name="text-changed-region" combine="choice">
+ <rng:element name="text:changed-region">
+ <rng:ref name="text-changed-region-attr"/>
+ <rng:ref name="text-changed-region-content"/>
+ <rng:optional>
+ <rng:ref name="text-changed-region-content"/>
+ </rng:optional>
+ </rng:element>
+ </rng:define>
</rng:grammar>
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index b48c9bbd44e8..ada8f45d34c1 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -1590,6 +1590,24 @@ void SwRangeRedline::CallDisplayFunc(size_t nMyPos)
ShowOriginal(0, nMyPos);
}
+namespace
+{
+RedlineType GetRedlineTypeIgnoringAdditonalFormat(const SwRangeRedline&
rRedline)
+{
+ RedlineType eType = rRedline.GetType();
+
+ if (eType == RedlineType::Format && rRedline.GetStackCount() > 1
+ && rRedline.GetType(1) == RedlineType::Delete)
+ {
+ // Consider format-on-delete the same as simple delete, so the range
gets moved to the
+ // "Deleted Change Tracking content" toplevel section from body
content during file save.
+ eType = RedlineType::Delete;
+ }
+
+ return eType;
+}
+}
+
void SwRangeRedline::Show(sal_uInt16 nLoop, size_t nMyPos, bool bForced)
{
SwDoc& rDoc = GetDoc();
@@ -1611,7 +1629,7 @@ void SwRangeRedline::Show(sal_uInt16 nLoop, size_t
nMyPos, bool bForced)
rDoc.getIDocumentRedlineAccess().SetRedlineFlags_intern(eOld |
RedlineFlags::Ignore);
::sw::UndoGuard const undoGuard(rDoc.GetIDocumentUndoRedo());
- switch( GetType() )
+ switch (GetRedlineTypeIgnoringAdditonalFormat(*this))
{
case RedlineType::Insert: // Content has been inserted
m_bIsVisible = true;
@@ -1651,7 +1669,7 @@ void SwRangeRedline::Hide(sal_uInt16 nLoop, size_t
nMyPos, bool /*bForced*/)
rDoc.getIDocumentRedlineAccess().SetRedlineFlags_intern(eOld |
RedlineFlags::Ignore);
::sw::UndoGuard const undoGuard(rDoc.GetIDocumentUndoRedo());
- switch( GetType() )
+ switch (GetRedlineTypeIgnoringAdditonalFormat(*this))
{
case RedlineType::Insert: // Content has been inserted
m_bIsVisible = true;
diff --git a/sw/source/core/unocore/unoredline.cxx
b/sw/source/core/unocore/unoredline.cxx
index 414bebbc1056..76a95fdcffe7 100644
--- a/sw/source/core/unocore/unoredline.cxx
+++ b/sw/source/core/unocore/unoredline.cxx
@@ -176,6 +176,22 @@ static uno::Sequence<beans::PropertyValue>
lcl_GetSuccessorProperties(const SwRa
const SwRedlineData* pNext = rRedline.GetRedlineData().Next();
if(pNext)
{
+ uno::Reference<text::XText> xRedlineText;
+ if (pNext->GetType() == RedlineType::Delete)
+ {
+ // Something on delete: produce the XText for the underlying
delete.
+ const SwNodeIndex* pNodeIdx = rRedline.GetContentIdx();
+ if (pNodeIdx
+ && (pNodeIdx->GetNode().EndOfSectionIndex() -
pNodeIdx->GetNode().GetIndex())
+ > SwNodeOffset(1))
+ {
+ // We have at least one node between the start and end node,
create the
+ // SwXRedlineText.
+ SwDoc& rDoc = rRedline.GetDoc();
+ xRedlineText = new SwXRedlineText(&rDoc, *pNodeIdx);
+ }
+ }
+
return
{
// GetAuthorString(n) walks the SwRedlineData* chain;
@@ -183,10 +199,11 @@ static uno::Sequence<beans::PropertyValue>
lcl_GetSuccessorProperties(const SwRa
comphelper::makePropertyValue(UNO_NAME_REDLINE_AUTHOR,
rRedline.GetAuthorString(1)),
comphelper::makePropertyValue(UNO_NAME_REDLINE_DATE_TIME,
pNext->GetTimeStamp().GetUNODateTime()),
comphelper::makePropertyValue(UNO_NAME_REDLINE_COMMENT,
pNext->GetComment()),
- comphelper::makePropertyValue(UNO_NAME_REDLINE_TYPE,
SwRedlineTypeToOUString(pNext->GetType()))
+ comphelper::makePropertyValue(UNO_NAME_REDLINE_TYPE,
SwRedlineTypeToOUString(pNext->GetType())),
+ comphelper::makePropertyValue(UNO_NAME_REDLINE_TEXT, xRedlineText)
};
}
- return uno::Sequence<beans::PropertyValue>(4);
+ return uno::Sequence<beans::PropertyValue>(5);
}
uno::Any SwXRedlinePortion::getPropertyValue( const OUString& rPropertyName )
diff --git a/xmloff/qa/unit/data/del-then-format.docx
b/xmloff/qa/unit/data/del-then-format.docx
new file mode 100644
index 000000000000..f458578f5568
Binary files /dev/null and b/xmloff/qa/unit/data/del-then-format.docx differ
diff --git a/xmloff/qa/unit/text.cxx b/xmloff/qa/unit/text.cxx
index 2ef9e2673360..ce11001b12cd 100644
--- a/xmloff/qa/unit/text.cxx
+++ b/xmloff/qa/unit/text.cxx
@@ -1286,6 +1286,27 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testNestedSpans)
CPPUNIT_ASSERT_EQUAL(awt::FontWeight::BOLD, fWeight);
}
+CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testDeleteThenFormatOdtExport)
+{
+ // Given a document that has a delete redline, and then a format redline
on top of it:
+ loadFromFile(u"del-then-format.docx");
+
+ // When exporting that to DOCX:
+ save(u"writer8"_ustr);
+
+ // Then make sure <text:changed-region> not only has the
<text:format-change> child, but also an
+ // <text:deletion> one, with text:
+ xmlDocUniquePtr pXmlDoc = parseExport(u"content.xml"_ustr);
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: 1
+ // - Actual : 0
+ // - XPath '//text:tracked-changes/text:changed-region[2]/text:deletion'
number of nodes is incorrect
+ // i.e. <text:insertion> was written for a delete-then-format.
+ assertXPath(pXmlDoc,
"//text:tracked-changes/text:changed-region[2]/text:deletion");
+ // Also the delete's text was in the body text.
+ assertXPath(pXmlDoc,
"//text:tracked-changes/text:changed-region[2]/text:deletion/text:p");
+}
+
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/text/XMLRedlineExport.cxx
b/xmloff/source/text/XMLRedlineExport.cxx
index 093040c3b15b..34e3ae5c5264 100644
--- a/xmloff/source/text/XMLRedlineExport.cxx
+++ b/xmloff/source/text/XMLRedlineExport.cxx
@@ -43,6 +43,7 @@
#include <xmloff/xmlexp.hxx>
#include <xmloff/xmluconv.hxx>
#include <unotools/securityoptions.hxx>
+#include <comphelper/sequenceashashmap.hxx>
using namespace ::com::sun::star;
@@ -361,7 +362,8 @@ void XMLRedlineExport::ExportChangedRegion(
aAny = rPropSet->getPropertyValue(u"RedlineText"_ustr);
Reference<XText> xText;
aAny >>= xText;
- if (xText.is())
+ // Avoid double export for format-on-delete: no write on format, only
on delete.
+ if (xText.is() && sType == u"Delete")
{
rExport.GetTextParagraphExport()->exportText(xText);
// default parameters: bProgress, bExportParagraph ???
@@ -380,15 +382,31 @@ void XMLRedlineExport::ExportChangedRegion(
// process change info
if (aSuccessorData.hasElements())
{
- // The only change that can be "undone" is an insertion -
- // after all, you can't re-insert a deletion, but you can
- // delete an insertion. This assumption is asserted in
- // ExportChangeInfo(Sequence<PropertyValue>&).
+ // Look up the type of the change. In practice this is always insert
or delete, other types
+ // can't have additional redlines on top of them.
+ OUString sType;
+ comphelper::SequenceAsHashMap aMap(aSuccessorData);
+ auto it = aMap.find("RedlineType");
+ if (it != aMap.end())
+ {
+ it->second >>= sType;
+ }
SvXMLElementExport aSecondChangeElem(
- rExport, XML_NAMESPACE_TEXT, XML_INSERTION,
+ rExport, XML_NAMESPACE_TEXT, ConvertTypeName(sType),
true, true);
ExportChangeInfo(aSuccessorData);
+ it = aMap.find("RedlineText");
+ if (it != aMap.end())
+ {
+ // Delete has its own content outside the body text: export it
here.
+ uno::Reference<text::XText> xText;
+ it->second >>= xText;
+ if (xText.is())
+ {
+ rExport.GetTextParagraphExport()->exportText(xText);
+ }
+ }
}
// else: no hierarchical change
}
@@ -529,10 +547,10 @@ void XMLRedlineExport::ExportChangeInfo(
}
else if( rVal.Name == "RedlineType" )
{
- // check if this is an insertion; cf. comment at calling location
+ // check if this has one of the expected types
OUString sTmp;
rVal.Value >>= sTmp;
- DBG_ASSERT(sTmp == "Insert",
+ DBG_ASSERT(sTmp == "Insert" || sTmp == "Delete",
"hierarchical change must be insertion");
}
// else: unknown value -> ignore
