sw/qa/extras/odfexport/odfexport.cxx | 6 +-
sw/qa/extras/ooxmlexport/ooxmlexport13.cxx | 4 -
sw/source/core/doc/doclay.cxx | 11 +++-
sw/source/core/doc/textboxhelper.cxx | 6 +-
sw/source/core/unocore/unotext.cxx | 21 +++++++--
writerfilter/source/dmapper/DomainMapper_Impl.cxx | 49 +++++++++++-----------
writerfilter/source/dmapper/GraphicImport.cxx | 11 ++--
7 files changed, 63 insertions(+), 45 deletions(-)
New commits:
commit 4d6243693c228703394c00164276f8326447beb9
Author: Vasily Melenchuk <vasily.melenc...@cib.de>
AuthorDate: Mon Dec 27 13:54:23 2021 +0300
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Aug 3 17:57:50 2022 +0200
tdf#143703 sw: always assign name for fly section
Previously generated name was assigned only if not in doc
reading mode. But there is no guarantee that it will be assigned
later. Better to insert any name in SwDoc::MakeFlySection_() and
later it can be overwritten, but fly will definitely have
any unique name.
* Some test failed because GraphicImport_Impl::applyName() overwrote the
name with a different generated one.
* This breaks chaining of VML shapes, see test testTDF87348.
The code introduced in commit 091fe76b6329b4bb974987554369cbfadd8f2401
in DomainMapper_Impl::ChainTextFrames() breaks if the text frame
already has a name; it's a bit confusing which names there come from
the file and which come from the API, and it also mixes 2 different
cases of VML chaining and DrawingML chaining that look like they
should be using different data.
* This also breaks moving flys anchored at-char in flys into them in
SwXText::convertToTextFrame(), see ooxmlexport13 testFlyInFly.
This kind of worked by accident before: the fly is copied and then the
original deleted, keeping the same name (with help of
SwDoc::mbCopyIsMove); with no name it would compare the SdrObject
pointer, which is different for the new copy, now the name is the
same.
Fix this by only moving flys anchored at the edge of the selection
back inoto the body; it turns out that Word actually supports at-char
anchors in text frames, but only if it's a VML shape or Compatibility
Mode or whatever; i wasn't able to do it in a document created from
scratch.
This is a bit tricky to ignore the nodes added for floating tables as
seen in ooxmlexport10 testFloatingTablesAnchor.
* Another change is required in SwDoc::SetFlyName() because of
testTdf127732, as it would rename a frame named "Frame1" to "Frame2"
when called to rename it to "Frame1".
* Some tests failed because after MakeFlySection_() assigns a name it is
immediately unconditionally overwritten; replace that with asserts
Change-Id: I46752a4413ba3a9e981eccd1e153b3aaf8053781
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127556
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/sw/qa/extras/odfexport/odfexport.cxx
b/sw/qa/extras/odfexport/odfexport.cxx
index e58b772b8cdf..3fb14b220c69 100644
--- a/sw/qa/extras/odfexport/odfexport.cxx
+++ b/sw/qa/extras/odfexport/odfexport.cxx
@@ -936,7 +936,7 @@ DECLARE_ODFEXPORT_TEST(testTdf134987, "tdf134987.docx")
OUString aMediaType;
// checking first object (formula)
{
- uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("1"), uno::UNO_QUERY);
+ uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("Object1"), uno::UNO_QUERY);
uno::Reference<lang::XComponent>
xObj(xEOSupplier->getEmbeddedObject());
CPPUNIT_ASSERT(xObj.is());
@@ -950,7 +950,7 @@ DECLARE_ODFEXPORT_TEST(testTdf134987, "tdf134987.docx")
}
// checking second object (chart)
{
- uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("2"), uno::UNO_QUERY);
+ uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("Object2"), uno::UNO_QUERY);
uno::Reference<lang::XComponent>
xObj(xEOSupplier->getEmbeddedObject());
CPPUNIT_ASSERT(xObj.is());
@@ -964,7 +964,7 @@ DECLARE_ODFEXPORT_TEST(testTdf134987, "tdf134987.docx")
}
// checking third object (chart)
{
- uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("3"), uno::UNO_QUERY);
+ uno::Reference<document::XEmbeddedObjectSupplier>
xEOSupplier(xAccess->getByName("Object3"), uno::UNO_QUERY);
uno::Reference<lang::XComponent>
xObj(xEOSupplier->getEmbeddedObject());
CPPUNIT_ASSERT(xObj.is());
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index 21d85580ecb9..751ebdcf8167 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -41,11 +41,11 @@ DECLARE_SW_EXPORT_TEST(testFlyInFly, "ooo39250-1-min.rtf",
nullptr, Test)
// check that anchor of text frame is in other text frame
uno::Reference<text::XTextContent> const xAnchored(getShape(3),
uno::UNO_QUERY);
CPPUNIT_ASSERT(xAnchored.is());
- CPPUNIT_ASSERT_EQUAL(OUString(""),
uno::Reference<container::XNamed>(xAnchored, uno::UNO_QUERY_THROW)->getName());
+ CPPUNIT_ASSERT_EQUAL(OUString("Frame1")/*generated name*/,
uno::Reference<container::XNamed>(xAnchored, uno::UNO_QUERY_THROW)->getName());
uno::Reference<text::XText> const
xAnchorText(xAnchored->getAnchor()->getText());
uno::Reference<text::XTextFrame> const xAnchorFrame(xAnchorText,
uno::UNO_QUERY);
CPPUNIT_ASSERT(xAnchorFrame.is());
- CPPUNIT_ASSERT_EQUAL(OUString("Frame2"),
uno::Reference<container::XNamed>(xAnchorFrame,
uno::UNO_QUERY_THROW)->getName());
+ CPPUNIT_ASSERT_EQUAL(OUString("Frame3"),
uno::Reference<container::XNamed>(xAnchorFrame,
uno::UNO_QUERY_THROW)->getName());
}
DECLARE_OOXMLEXPORT_TEST(testTdf125778_lostPageBreakTOX,
"tdf125778_lostPageBreakTOX.docx")
diff --git a/sw/source/core/doc/doclay.cxx b/sw/source/core/doc/doclay.cxx
index 8b9cd0ee6484..ca49fdc781b5 100644
--- a/sw/source/core/doc/doclay.cxx
+++ b/sw/source/core/doc/doclay.cxx
@@ -163,13 +163,12 @@ SwFlyFrameFormat* SwDoc::MakeFlySection_( const
SwPosition& rAnchPos,
pFrameFormat = getIDocumentStylePoolAccess().GetFrameFormatFromPool(
RES_POOLFRM_FRAME );
OUString sName;
- if( !mbInReading )
- switch( rNode.GetNodeType() )
- {
+ switch( rNode.GetNodeType() )
+ {
case SwNodeType::Grf: sName = GetUniqueGrfName(); break;
case SwNodeType::Ole: sName = GetUniqueOLEName(); break;
default: sName = GetUniqueFrameName(); break;
- }
+ }
SwFlyFrameFormat* pFormat = MakeFlyFrameFormat( sName, pFrameFormat );
// Create content and connect to the format.
@@ -1416,6 +1415,10 @@ const SwFlyFrameFormat* SwDoc::FindFlyByName( const
OUString& rName, SwNodeType
void SwDoc::SetFlyName( SwFlyFrameFormat& rFormat, const OUString& rName )
{
+ if (rFormat.GetName() == rName)
+ {
+ return;
+ }
OUString sName( rName );
if( sName.isEmpty() || FindFlyByName( sName ) )
{
diff --git a/sw/source/core/doc/textboxhelper.cxx
b/sw/source/core/doc/textboxhelper.cxx
index e6875e982942..12d7405d78fb 100644
--- a/sw/source/core/doc/textboxhelper.cxx
+++ b/sw/source/core/doc/textboxhelper.cxx
@@ -132,7 +132,8 @@ void SwTextBoxHelper::create(SwFrameFormat* pShape,
SdrObject* pObject, bool bCo
xPropertySet->setPropertyValue(UNO_NAME_SURROUND,
uno::Any(text::WrapTextMode_THROUGH));
uno::Reference<container::XNamed> xNamed(xTextFrame, uno::UNO_QUERY);
- xNamed->setName(pShape->GetDoc()->GetUniqueFrameName());
+ assert(!xNamed->getName().isEmpty());
+ (void)xNamed;
// Link its text range to the original shape.
uno::Reference<text::XTextRange> xTextBox(xTextFrame,
uno::UNO_QUERY_THROW);
@@ -240,7 +241,8 @@ void SwTextBoxHelper::set(SwFrameFormat* pShapeFormat,
SdrObject* pObj,
xPropertySet->setPropertyValue(UNO_NAME_SURROUND,
uno::Any(text::WrapTextMode_THROUGH));
// Add a new name to it
uno::Reference<container::XNamed> xNamed(xNew, uno::UNO_QUERY);
- xNamed->setName(pShapeFormat->GetDoc()->GetUniqueFrameName());
+ assert(!xNamed->getName().isEmpty());
+ (void)xNamed;
// And sync. properties.
uno::Reference<drawing::XShape> xShape(pObj->getUnoShape(),
uno::UNO_QUERY);
syncProperty(pShapeFormat, RES_FRM_SIZE, MID_FRMSIZE_SIZE,
uno::Any(xShape->getSize()), pObj);
diff --git a/sw/source/core/unocore/unotext.cxx
b/sw/source/core/unocore/unotext.cxx
index 40ddae537e8c..6b400f6f3ea1 100644
--- a/sw/source/core/unocore/unotext.cxx
+++ b/sw/source/core/unocore/unotext.cxx
@@ -1618,6 +1618,8 @@ SwXText::convertToTextFrame(
}
bool bParaAfterInserted = false;
bool bParaBeforeInserted = false;
+ ::std::optional<SwPaM> oAnchorCheckPam;
+ oAnchorCheckPam.emplace(*pStartPam->Start(), *pEndPam->End());
if (
pStartStartNode && pEndStartNode &&
(pStartStartNode != pEndStartNode || pStartStartNode != GetStartNode())
@@ -1698,6 +1700,7 @@ SwXText::convertToTextFrame(
bParaAfterInserted =
GetDoc()->getIDocumentContentOperations().AppendTextNode( aEnd );
pEndPam->DeleteMark();
*pEndPam->GetPoint() = aEnd;
+ *oAnchorCheckPam->End() = aEnd;
}
pStartPam->SetMark();
*pStartPam->End() = *pEndPam->End();
@@ -1712,10 +1715,17 @@ SwXText::convertToTextFrame(
{
const SwFrameFormat* pFrameFormat =
(*m_pImpl->m_pDoc->GetSpzFrameFormats())[i];
const SwFormatAnchor& rAnchor = pFrameFormat->GetAnchor();
- if ( !isGraphicNode(pFrameFormat) &&
- (RndStdIds::FLY_AT_PARA == rAnchor.GetAnchorId() ||
RndStdIds::FLY_AT_CHAR == rAnchor.GetAnchorId()) &&
- pStartPam->Start()->GetNodeIndex() <=
rAnchor.GetContentAnchor()->GetNodeIndex() &&
- pStartPam->End()->GetNodeIndex() >=
rAnchor.GetContentAnchor()->GetNodeIndex())
+ // note: Word can do at-char anchors in text frames - sometimes!
+ // see testFlyInFly for why this checks only the edges of the
selection,
+ // and testFloatingTablesAnchor for why it excludes pre/post table
+ // added nodes
+ if (!isGraphicNode(pFrameFormat)
+ && ( (RndStdIds::FLY_AT_PARA == rAnchor.GetAnchorId()
+ && ( oAnchorCheckPam->Start()->nNode.GetIndex() ==
rAnchor.GetContentAnchor()->nNode.GetIndex()
+ || oAnchorCheckPam->End()->nNode.GetIndex() ==
rAnchor.GetContentAnchor()->nNode.GetIndex()))
+ || (RndStdIds::FLY_AT_CHAR == rAnchor.GetAnchorId()
+ && ( *oAnchorCheckPam->Start() ==
*rAnchor.GetContentAnchor()
+ || *oAnchorCheckPam->End() ==
*rAnchor.GetContentAnchor()))))
{
if (pFrameFormat->GetName().isEmpty())
{
@@ -1727,6 +1737,7 @@ SwXText::convertToTextFrame(
}
}
}
+ oAnchorCheckPam.reset(); // clear SwIndex before deleting nodes
const uno::Reference<text::XTextFrame> xNewFrame(
SwXTextFrame::CreateXTextFrame(*m_pImpl->m_pDoc, nullptr));
@@ -1744,7 +1755,7 @@ SwXText::convertToTextFrame(
new SwXTextRange(*pStartPam, this);
assert(rNewFrame.IsDescriptor());
rNewFrame.attachToRange(xInsertTextRange, pStartPam.get());
- rNewFrame.setName(m_pImpl->m_pDoc->GetUniqueFrameName());
+ assert(!rNewFrame.getName().isEmpty());
}
SwTextNode *const pTextNode(pStartPam->GetNode().GetTextNode());
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index f5ca32d6f6f6..3051323c8015 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -4656,14 +4656,15 @@ void DomainMapper_Impl::ChainTextFrames()
sal_Int32 nId;
sal_Int32 nSeq;
OUString s_mso_next_textbox;
- bool bShapeNameSet;
- TextFramesForChaining(): nId(0), nSeq(0), bShapeNameSet(false) {}
+ OUString shapeName;
+ TextFramesForChaining() : nId(0), nSeq(0) {}
} ;
typedef std::map <OUString, TextFramesForChaining> ChainMap;
try
{
ChainMap aTextFramesForChainingHelper;
+ ::std::vector<TextFramesForChaining> chainingWPS;
OUString sChainNextName("ChainNextName");
//learn about ALL of the textboxes and their chaining values first -
because frames are processed in no specific order.
@@ -4701,19 +4702,22 @@ void DomainMapper_Impl::ChainTextFrames()
//Sometimes the shape names have not been imported. If not, we
may have a fallback name.
//Set name later, only if required for linking.
- if( sShapeName.isEmpty() )
- aChainStruct.bShapeNameSet = false;
- else
- {
- aChainStruct.bShapeNameSet = true;
- sLinkChainName = sShapeName;
- }
+ aChainStruct.shapeName = sShapeName;
- if( !sLinkChainName.isEmpty() )
+ if (!sLinkChainName.isEmpty())
{
aChainStruct.xShape = rTextFrame;
aTextFramesForChainingHelper[sLinkChainName] = aChainStruct;
}
+ if (aChainStruct.s_mso_next_textbox.isEmpty())
+ { // no VML chaining => try to chain DrawingML via IDs
+ aChainStruct.xShape = rTextFrame;
+ if (!sLinkChainName.isEmpty())
+ { // for member of group shapes, TestTdf73499
+ aChainStruct.shapeName = sLinkChainName;
+ }
+ chainingWPS.emplace_back(aChainStruct);
+ }
}
//if mso-next-textbox tags are provided, create those vml-style links
first. Afterwards we will make dml-style id/seq links.
@@ -4728,22 +4732,22 @@ void DomainMapper_Impl::ChainTextFrames()
if( nextFinder != aTextFramesForChainingHelper.end() )
{
//if the frames have no name yet, then set them.
LinkDisplayName / ChainName are read-only.
- if( !msoItem.second.bShapeNameSet )
+ if (msoItem.second.shapeName.isEmpty())
{
uno::Reference< container::XNamed > xNamed(
msoItem.second.xShape, uno::UNO_QUERY );
if ( xNamed.is() )
{
xNamed->setName( msoItem.first );
- msoItem.second.bShapeNameSet = true;
+ msoItem.second.shapeName = msoItem.first;
}
}
- if( !nextFinder->second.bShapeNameSet )
+ if (nextFinder->second.shapeName.isEmpty())
{
uno::Reference< container::XNamed > xNamed(
nextFinder->second.xShape, uno::UNO_QUERY );
if ( xNamed.is() )
{
xNamed->setName( nextFinder->first );
- nextFinder->second.bShapeNameSet = true;
+ nextFinder->second.shapeName = msoItem.first;
}
}
@@ -4751,7 +4755,7 @@ void DomainMapper_Impl::ChainTextFrames()
uno::Reference<beans::XPropertySet>
xPropertySet(xTextContent, uno::UNO_QUERY);
//The reverse chaining happens automatically, so only one
direction needs to be set
- xPropertySet->setPropertyValue(sChainNextName,
uno::Any(nextFinder->first));
+ xPropertySet->setPropertyValue(sChainNextName,
uno::Any(nextFinder->second.shapeName));
//the last item in an mso-next-textbox chain is
indistinguishable from id/seq items. Now that it is handled, remove it.
if( nextFinder->second.s_mso_next_textbox.isEmpty() )
@@ -4764,26 +4768,23 @@ void DomainMapper_Impl::ChainTextFrames()
const sal_Int32 nDirection = 1;
//Finally - go through and attach the chains based on matching ID and
incremented sequence number (dml-style).
- for (const auto& rOuter : aTextFramesForChainingHelper)
+ for (const auto& rOuter : chainingWPS)
{
- if( rOuter.second.s_mso_next_textbox.isEmpty() ) //non-empty ones
already handled earlier - so skipping them now.
- {
- for (const auto& rInner : aTextFramesForChainingHelper)
+ for (const auto& rInner : chainingWPS)
{
- if ( rInner.second.nId == rOuter.second.nId )
+ if (rInner.nId == rOuter.nId)
{
- if ( rInner.second.nSeq == ( rOuter.second.nSeq +
nDirection ) )
+ if (rInner.nSeq == (rOuter.nSeq + nDirection))
{
- uno::Reference<text::XTextContent>
xTextContent(rOuter.second.xShape, uno::UNO_QUERY_THROW);
+ uno::Reference<text::XTextContent> const
xTextContent(rOuter.xShape, uno::UNO_QUERY_THROW);
uno::Reference<beans::XPropertySet>
xPropertySet(xTextContent, uno::UNO_QUERY);
//The reverse chaining happens automatically, so
only one direction needs to be set
- xPropertySet->setPropertyValue(sChainNextName,
uno::Any(rInner.first));
+ xPropertySet->setPropertyValue(sChainNextName,
uno::Any(rInner.shapeName));
break ; //there cannot be more than one next frame
}
}
}
- }
}
m_vTextFramesForChaining.clear(); //clear the vector
}
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx
b/writerfilter/source/dmapper/GraphicImport.cxx
index f3b145a947d5..4cdfacc8fb46 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -405,11 +405,12 @@ public:
{
try
{
- // Ask the graphic naming helper to find out the name for this
- // object: It's around till the end of the import, so it remembers
- // what's the first free name.
- uno::Reference< container::XNamed > xNamed(
xGraphicObjectProperties, uno::UNO_QUERY_THROW );
-
xNamed->setName(rDomainMapper.GetGraphicNamingHelper().NameGraphic(sName));
+ if (!sName.isEmpty())
+ {
+ uno::Reference<container::XNamed> const
xNamed(xGraphicObjectProperties, uno::UNO_QUERY_THROW);
+ xNamed->setName(sName);
+ }
+ // else: name is automatically generated by
SwDoc::MakeFlySection_()
xGraphicObjectProperties->setPropertyValue(getPropertyName(
PROP_DESCRIPTION ),
uno::Any( sAlternativeText ));
