writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx | 45 ++++++
writerfilter/qa/cppunittests/dmapper/data/redlined-shape-sdt.docx |binary
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 75
+++++-----
writerfilter/source/dmapper/DomainMapper_Impl.hxx | 1
4 files changed, 88 insertions(+), 33 deletions(-)
New commits:
commit b973805565be06aac4422fbb7af530fe8ee04bd0
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Feb 21 10:22:12 2024 +0100
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Thu Feb 22 17:17:57 2024 +0100
tdf#159815 DOCX import: fix redlined to-char image followed by inline SDT
The 2nd para of the bugdoc has a leading word, then a redlined anchored
shape, finally a redlined checkbox content control. The Writer import
result's content control starts at para start, while it should only
start after the anchored shape.
What happens is that writerfilter::dmapper::DomainMapper_Impl::PushSdt()
gets called to remember the SDT start, then
DomainMapper_Impl::applyToggleAttributes() deletes some content since
commit 8726cf692299ea262a7455adcf6ec25451c7869d (tdf#142700 DOCX: fix
lost track changes of images, 2021-07-08), which invalidates the SDT
start position, finally PopSdt gets called, but that fails because our
start position is no longer valid. This used to abort the import
process. Since commit 1b0f67018fa1d514ebca59e081efdd24c1d7811b (docx
import: correct redline content-controls, 2024-02-20), the import
finishes the but start of the SDT is incorrect: it's at the para start,
while it should start in the middle of the paragraph. The direct reason
for the invalidation is a call to SwXTextRange::Impl::Notify(), which is
from a content deletion from applyToggleAttributes().
Fix the problem by not deleting content when we're past PushSdt() and we
haven't called PopSdt(): extract the redlined anchored shape code into a
new DomainMapper_Impl::MergeAtContentImageRedlineWithNext() and call
that also in DomainMapper_Impl::PushSdt() early, so it won't be called
when we're in the middle of an SDT.
This way createTextCursorByRange() should not fail, so warn in case it
still fails in DomainMapper_Impl::PopSdt().
Change-Id: Ic4198804a92088ec268203d44c0da2d6997754b7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163678
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163689
diff --git a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
index d0693746f24b..16aa5cbfb2df 100644
--- a/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/qa/cppunittests/dmapper/DomainMapper_Impl.cxx
@@ -366,6 +366,51 @@ CPPUNIT_TEST_FIXTURE(Test, testFloattableSectend)
// i.e. the first table was lost.
CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(2), xTables->getCount());
}
+
+CPPUNIT_TEST_FIXTURE(Test, testRedlinedShapeThenSdt)
+{
+ // Given a file with a second paragraph where text is followed by a
redline, then an SDT:
+ // When importing that document:
+ loadFromFile(u"redlined-shape-sdt.docx");
+
+ // Then make sure the content control doesn't start at para start:
+ uno::Reference<text::XTextDocument> xTextDocument(mxComponent,
uno::UNO_QUERY);
+ uno::Reference<container::XEnumerationAccess>
xParaEnumAccess(xTextDocument->getText(),
+
uno::UNO_QUERY);
+ uno::Reference<container::XEnumeration> xParaEnum =
xParaEnumAccess->createEnumeration();
+ xParaEnum->nextElement();
+ uno::Reference<container::XEnumerationAccess>
xPortionEnumAccess(xParaEnum->nextElement(),
+
uno::UNO_QUERY);
+ uno::Reference<container::XEnumeration> xPortionEnum =
xPortionEnumAccess->createEnumeration();
+
+ uno::Reference<beans::XPropertySet> xPortion(xPortionEnum->nextElement(),
uno::UNO_QUERY);
+ // Without the accompanying fix in place, this test would have failed with:
+ // - Expected: Text
+ // - Actual : ContentControl
+ // i.e. the content control started at para start.
+ CPPUNIT_ASSERT_EQUAL(u"Text"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ // Redline start+end pair, containing a pair of text portions with an
anchored object in the
+ // middle.
+ CPPUNIT_ASSERT_EQUAL(u"Redline"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(u"Text"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(u"Frame"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(u"Text"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(u"Redline"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+ xPortion.set(xPortionEnum->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(u"ContentControl"_ustr,
+
xPortion->getPropertyValue("TextPortionType").get<OUString>());
+}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/qa/cppunittests/dmapper/data/redlined-shape-sdt.docx
b/writerfilter/qa/cppunittests/dmapper/data/redlined-shape-sdt.docx
new file mode 100644
index 000000000000..ea7f4a5bd664
Binary files /dev/null and
b/writerfilter/qa/cppunittests/dmapper/data/redlined-shape-sdt.docx differ
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 0c0fa255d896..a76151096604 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1003,6 +1003,10 @@ void DomainMapper_Impl::PushSdt()
return;
}
+ // This may delete text, so call it early, before we would set our start
position, which may be
+ // invalidated by a delete.
+ MergeAtContentImageRedlineWithNext(xTextAppend);
+
uno::Reference<text::XText> xText = xTextAppend->getText();
if (!xText.is())
{
@@ -1041,6 +1045,7 @@ void DomainMapper_Impl::PopSdt()
}
catch (const uno::RuntimeException&)
{
+ TOOLS_WARN_EXCEPTION("writerfilter",
"DomainMapper_Impl::DomainMapper_Impl::PopSdt: createTextCursorByRange()
failed");
// We redline form controls and that gets us confused when
// we process the SDT around the placeholder. What seems to
// happen is we lose the text-range when we pop the SDT position.
@@ -3199,6 +3204,42 @@ void DomainMapper_Impl::applyToggleAttributes(const
PropertyMapPtr& pPropertyMap
}
}
+void DomainMapper_Impl::MergeAtContentImageRedlineWithNext(const
css::uno::Reference<css::text::XTextAppend>& xTextAppend)
+{
+ // remove workaround for change tracked images, if they are part of a
redline,
+ // i.e. if the next run is a tracked change with the same type, author and
date,
+ // as in the change tracking of the image.
+ if ( m_bRedlineImageInPreviousRun )
+ {
+ auto pCurrentRedline = m_aRedlines.top().size() > 0
+ ? m_aRedlines.top().back()
+ : GetTopContextOfType(CONTEXT_CHARACTER) &&
+ GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().size() > 0
+ ? GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().back()
+ : nullptr;
+ if ( m_previousRedline && pCurrentRedline &&
+ // same redline
+ (m_previousRedline->m_nToken & 0xffff) ==
(pCurrentRedline->m_nToken & 0xffff) &&
+ m_previousRedline->m_sAuthor == pCurrentRedline->m_sAuthor &&
+ m_previousRedline->m_sDate == pCurrentRedline->m_sDate )
+ {
+ uno::Reference< text::XTextCursor > xCursor =
xTextAppend->getEnd()->getText( )->createTextCursor( );
+ assert(xCursor.is());
+ xCursor->gotoEnd(false);
+ xCursor->goLeft(2, true);
+ if ( xCursor->getString() == u"" )
+ {
+ xCursor->goRight(1, true);
+ xCursor->setString("");
+ xCursor->gotoEnd(false);
+ xCursor->goLeft(1, true);
+ xCursor->setString("");
+ }
+ }
+
+ m_bRedlineImageInPreviousRun = false;
+ }
+}
void DomainMapper_Impl::appendTextPortion( const OUString& rString, const
PropertyMapPtr& pPropertyMap )
{
@@ -3228,39 +3269,7 @@ void DomainMapper_Impl::applyToggleAttributes(const
PropertyMapPtr& pPropertyMap
rValue.Value <<= false;
}
- // remove workaround for change tracked images, if they are part of a
redline,
- // i.e. if the next run is a tracked change with the same type, author
and date,
- // as in the change tracking of the image.
- if ( m_bRedlineImageInPreviousRun )
- {
- auto pCurrentRedline = m_aRedlines.top().size() > 0
- ? m_aRedlines.top().back()
- : GetTopContextOfType(CONTEXT_CHARACTER) &&
-
GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().size() > 0
- ?
GetTopContextOfType(CONTEXT_CHARACTER)->Redlines().back()
- : nullptr;
- if ( m_previousRedline && pCurrentRedline &&
- // same redline
- (m_previousRedline->m_nToken & 0xffff) ==
(pCurrentRedline->m_nToken & 0xffff) &&
- m_previousRedline->m_sAuthor == pCurrentRedline->m_sAuthor
&&
- m_previousRedline->m_sDate == pCurrentRedline->m_sDate )
- {
- uno::Reference< text::XTextCursor > xCursor =
xTextAppend->getEnd()->getText( )->createTextCursor( );
- assert(xCursor.is());
- xCursor->gotoEnd(false);
- xCursor->goLeft(2, true);
- if ( xCursor->getString() == u"" )
- {
- xCursor->goRight(1, true);
- xCursor->setString("");
- xCursor->gotoEnd(false);
- xCursor->goLeft(1, true);
- xCursor->setString("");
- }
- }
-
- m_bRedlineImageInPreviousRun = false;
- }
+ MergeAtContentImageRedlineWithNext(xTextAppend);
uno::Reference< text::XTextRange > xTextRange;
if (m_aTextAppendStack.top().xInsertPosition.is())
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 082eda8fc5c9..7db3dbcddcae 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -793,6 +793,7 @@ public:
void finishParagraph( const PropertyMapPtr& pPropertyMap, const bool
bRemove = false, const bool bNoNumbering = false);
void applyToggleAttributes( const PropertyMapPtr& pPropertyMap );
+ void MergeAtContentImageRedlineWithNext(const
css::uno::Reference<css::text::XTextAppend>& xTextAppend);
void appendTextPortion( const OUString& rString, const PropertyMapPtr&
pPropertyMap );
void appendTextContent(const
css::uno::Reference<css::text::XTextContent>&, const
css::uno::Sequence<css::beans::PropertyValue>&);
void appendOLE( const OUString& rStreamName, const
std::shared_ptr<OLEHandler>& pOleHandler );
