sw/inc/textboxhelper.hxx | 40 ++ sw/qa/extras/layout/layout.cxx | 5 sw/qa/extras/ooxmlexport/data/tdf149546.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport14.cxx | 6 sw/source/core/doc/docfly.cxx | 7 sw/source/core/doc/textboxhelper.cxx | 310 ++++++++++++++-------- sw/source/core/edit/edundo.cxx | 9 sw/source/core/layout/atrfrm.cxx | 9 sw/source/core/text/porfly.cxx | 7 sw/source/core/undo/undobj1.cxx | 12 sw/source/core/unocore/unodraw.cxx | 22 + sw/source/core/unocore/unotext.cxx | 23 + writerfilter/source/dmapper/DomainMapper_Impl.cxx | 27 + 13 files changed, 349 insertions(+), 128 deletions(-)
New commits: commit d506ca99d9ecbedadc7d6a83669313eca78768c0 Author: Mike Kaganski <mike.kagan...@collabora.com> AuthorDate: Tue Jan 31 11:57:38 2023 +0300 Commit: Mike Kaganski <mike.kagan...@collabora.com> CommitDate: Tue Jan 31 12:32:37 2023 +0000 tdf#153289: remove problematic assertion It is unclear what it should guarantee; but at least when ungrouping, SwDoc::UnGroupSelection copy-constructs a shared pointer (increasing the refcount), then a copy-constructed argument passed to lcl_CollectTextBoxesForSubGroupObj increases it once again, and then the assertion expectedly fails. Change-Id: I0cb5f303c67b2dc67d5583a9eb03fe405af3573d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/146327 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> diff --git a/sw/source/core/doc/textboxhelper.cxx b/sw/source/core/doc/textboxhelper.cxx index 97faf3a19a70..c69f359d6be2 100644 --- a/sw/source/core/doc/textboxhelper.cxx +++ b/sw/source/core/doc/textboxhelper.cxx @@ -1710,7 +1710,6 @@ SwFrameFormat* SwTextBoxNode::GetTextBox(const SdrObject* pDrawObject) const if (size_t(pTextBoxes.use_count()) != pTextBoxes->GetTextBoxCount() + size_t(1)) { SAL_WARN("sw.core", "SwTextBoxNode::GetTextBox(): RefCount and TexBox count mismatch!"); - assert(false); } } commit 05eaee071ba6eb93ccc4da31b11ee01d102a960c Author: Attila Bakos (NISZ) <bakos.attilakar...@nisz.hu> AuthorDate: Mon Jun 20 17:27:53 2022 +0200 Commit: Mike Kaganski <mike.kagan...@collabora.com> CommitDate: Tue Jan 31 12:32:30 2023 +0000 tdf#148687 tdf#149173 tdf#149546 sw: fix crash with textboxes Regression from 2110597ac730fa07dbbdd603fda82b182ed27c9e "tdf#147485 sw: fix group shape crash using std::shared_ptr". Details: 1) Using reference instead of copy assignment in SwTextBoxHelper. 2) Cleanup: Unused parts of SwTextBoxHelper were removed. 3) Fixing destruction of textboxes, in case when first the shape is removed, with clearing all textboxes from the doc and the shape before the pointer is released. All of this only have to be done if the call is not coming from the swdoc dtor, unless there will be double freeing. 4) Missing style conversion was fixed in writerfilter. 5) Don't import sections in textboxes, unless the hack of dummy paragraph before tables in sections will be applied and the paragraph with anchored objects inside will be removed with the objects; 6) ConvertTextRangeToTextFrame also fixed, so embed frames in frames are imported from now. (Also textboxes in frames, this can be useful when there is a floating table having group textbox with nested complex content inside, or floating table in floating table, etc...) Note: Follow up commit will enable group textbox import in frames and tables. 7) Import of group textboxes with complex content in header/footer was impossible, from now this is also supported both from DOCX and ODT (test included). 8) Guard class for blocking unwanted recursive textbox sync has been introduced, and maybe some speedup with group textbox import has been achieved. 9) The anchor sync method got a new function which avoids unnecessary sync when the anchor is the same. 10) Sync of As_char textboxes during layout calculation caused crash so that has a workaround from now, for DOCX import anchor change and Undo. That syncs starts before the layout calculation so sync not needed later. 11) A memory leak was found in Undo, which has been fixed. Note: layout test "testTdf147485Forcepoint" has to be limited to Windows and Mac builds, because font substitution causes crash on the Unix based systems. Change-Id: I69d5d79cc120e3a70ba9285be32ec36a434b2a04 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/136192 Tested-by: László Németh <nem...@numbertext.org> Reviewed-by: László Németh <nem...@numbertext.org> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/143373 Tested-by: Mike Kaganski <mike.kagan...@collabora.com> Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> diff --git a/sw/inc/textboxhelper.hxx b/sw/inc/textboxhelper.hxx index b0ab5618b466..991b4a5c77f5 100644 --- a/sw/inc/textboxhelper.hxx +++ b/sw/inc/textboxhelper.hxx @@ -180,6 +180,9 @@ public: /// vector filled with the textboxes. static std::vector<SwFrameFormat*> CollectTextBoxes(SdrObject* pGroupObject, SwFrameFormat* pFormat); + + // Compares the anchor of the first and second given formats, and decides whether sync needed. + static bool isAnchorSyncNeeded(const SwFrameFormat* pFirst, const SwFrameFormat* pSecond); }; /// Textboxes are basically textframe + shape pairs. This means one shape has one frame. @@ -188,6 +191,8 @@ public: /// it can have multiple textboxes. class SwTextBoxNode { + friend class SwTextBoxLockGuard; + // One TextBox-entry struct SwTextBoxElement { @@ -195,8 +200,6 @@ class SwTextBoxNode SwFrameFormat* m_pTextBoxFormat; // The Draw object where the textbox belongs to SdrObject* m_pDrawObject; - // This is for indicating if the textbox is in special case: for example during undo. - bool m_bIsActive; }; // This vector stores the textboxes what belongs to this node @@ -205,8 +208,12 @@ class SwTextBoxNode // (and the textboxes) SwFrameFormat* m_pOwnerShapeFormat; + // Prevents oscillating during recursive clone calling. mutable bool m_bIsCloningInProgress; + // Protection against looping + bool m_bLock; + public: // Not needed. SwTextBoxNode() = delete; @@ -239,22 +246,24 @@ public: // to the given shape (pDrawObject) SwFrameFormat* GetTextBox(const SdrObject* pDrawObject) const; - // Is this textbox has special state, undo for example? - bool IsTextBoxActive(const SdrObject* pDrawObject) const; - - // Setters for the state flag. - void SetTextBoxInactive(const SdrObject* pDrawObject); - void SetTextBoxActive(const SdrObject* pDrawObject); + // Clears all textboxes of this node from the doc and also from here. + void ClearAll(); // If this is a group shape, that returns true. bool IsGroupTextBox() const; + // This returns with the shape what this class belongs to. SwFrameFormat* GetOwnerShape() { return m_pOwnerShapeFormat; }; + // This will give the current number of textboxes. size_t GetTextBoxCount() const { return m_pTextBoxes.size(); }; + // Returns with a const collection of textboxes owned by this node. std::map<SdrObject*, SwFrameFormat*> GetAllTextBoxes() const; + // Does the copy, and assign of all textboxes of this node to the given format. + // Important: The given format has to be a shape-format, and must have same structure + // as the owner shape has. If the structure different, the cloning will be aborted. void Clone(SwDoc* pDoc, const SwFormatAnchor& rNewAnc, SwFrameFormat* o_pTarget, bool bSetAttr, bool bMakeFrame) const; @@ -264,6 +273,21 @@ private: bool bMakeFrame) const; }; +// Helper class for preventing unwanted sync calls. +class SwTextBoxLockGuard +{ + SwTextBoxNode& m_rTextBoxes; + +public: + SwTextBoxLockGuard(SwTextBoxNode& rTextBoxes) + : m_rTextBoxes(rTextBoxes) + { + m_rTextBoxes.m_bLock = true; + } + + ~SwTextBoxLockGuard() { m_rTextBoxes.m_bLock = false; } +}; + #endif // INCLUDED_SW_INC_TEXTBOXHELPER_HXX /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/qa/extras/layout/layout.cxx b/sw/qa/extras/layout/layout.cxx index 08e33c87980c..2d57ba8642e2 100644 --- a/sw/qa/extras/layout/layout.cxx +++ b/sw/qa/extras/layout/layout.cxx @@ -2560,11 +2560,16 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testForcepoint103) createSwWebDoc(DATA_DIRECTORY, "forcepoint103.html"); } +// FIXME: The font substitution of the bugdoc causes crash in Linux builds, +// in addition this example file originally was a docx so check system type +// until that issue is not fixed: +#ifndef SAL_UNX //just care it doesn't crash/assert CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf147485Forcepoint) { createSwDoc(DATA_DIRECTORY, "tdf147485-forcepoint.doc"); } +#endif CPPUNIT_TEST_FIXTURE(SwLayoutWriter, testTdf118058) { diff --git a/sw/qa/extras/ooxmlexport/data/tdf149546.docx b/sw/qa/extras/ooxmlexport/data/tdf149546.docx new file mode 100644 index 000000000000..2811ec1898b4 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf149546.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx index d3feaf95f720..ae386767c584 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx @@ -849,6 +849,12 @@ CPPUNIT_TEST_FIXTURE(SwModelTestBase, testTdf147485) load(DATA_DIRECTORY, "Tdf147485.docx"); } +CPPUNIT_TEST_FIXTURE(SwModelTestBase, testTdf149546) +{ + // Before the fix this was impossible. + load(DATA_DIRECTORY, "tdf149546.docx"); +} + CPPUNIT_TEST_FIXTURE(SwModelTestBase, testUserField) { // Create an in-memory empty document with a user field. diff --git a/sw/source/core/doc/docfly.cxx b/sw/source/core/doc/docfly.cxx index db9518b2cecd..6dfc19c48f2d 100644 --- a/sw/source/core/doc/docfly.cxx +++ b/sw/source/core/doc/docfly.cxx @@ -920,11 +920,10 @@ bool SwDoc::ChgAnchor( const SdrMarkList& _rMrkList, pNd->InsertItem( aFormat, aPos.nContent.GetIndex(), 0 ); // Has a textbox attached to the format? Sync it as well! - if (SwTextBoxHelper::getOtherTextBoxFormat(pContact->GetFormat(), - RES_DRAWFRMFMT)) + if (pContact->GetFormat() && pContact->GetFormat()->GetOtherTextBoxFormats()) { - SwTextBoxHelper::syncFlyFrameAttr(*pContact->GetFormat(), - pContact->GetFormat()->GetAttrSet(), pObj); + SwTextBoxHelper::synchronizeGroupTextBoxProperty( + SwTextBoxHelper::changeAnchor, pContact->GetFormat(), pObj); } } break; diff --git a/sw/source/core/doc/textboxhelper.cxx b/sw/source/core/doc/textboxhelper.cxx index bb1d6f331dbc..97faf3a19a70 100644 --- a/sw/source/core/doc/textboxhelper.cxx +++ b/sw/source/core/doc/textboxhelper.cxx @@ -64,8 +64,6 @@ void SwTextBoxHelper::create(SwFrameFormat* pShape, SdrObject* pObject, bool bCo assert(pShape); assert(pObject); - const bool bIsGroupObj = dynamic_cast<SdrObjGroup*>(pObject->getParentSdrObjectFromSdrObject()); - // If TextBox wasn't enabled previously if (pShape->GetOtherTextBoxFormats() && pShape->GetOtherTextBoxFormats()->GetTextBox(pObject)) return; @@ -113,9 +111,8 @@ void SwTextBoxHelper::create(SwFrameFormat* pShape, SdrObject* pObject, bool bCo } else { - auto pTextBox = pShape->GetOtherTextBoxFormats(); + auto& pTextBox = pShape->GetOtherTextBoxFormats(); pTextBox->AddTextBox(pObject, pFormat); - pShape->SetOtherTextBoxFormats(pTextBox); pFormat->SetOtherTextBoxFormats(pTextBox); } // Initialize properties. @@ -178,8 +175,8 @@ void SwTextBoxHelper::create(SwFrameFormat* pShape, SdrObject* pObject, bool bCo if (xShapePropertySet->getPropertyValue(UNO_NAME_TEXT_WRITINGMODE) >>= eMode) syncProperty(pShape, RES_FRAMEDIR, 0, uno::makeAny(sal_Int16(eMode)), pObject); - if (bIsGroupObj) - doTextBoxPositioning(pShape, pObject); + changeAnchor(pShape, pObject); + syncTextBoxSize(pShape, pObject); // Check if the shape had text before and move it to the new textframe if (!bCopyText || sCopyableText.isEmpty()) @@ -210,35 +207,13 @@ void SwTextBoxHelper::set(SwFrameFormat* pShapeFormat, SdrObject* pObj, pFormat = pTextFrame->GetFrameFormat(); if (!pFormat) return; - std::vector<std::pair<beans::Property, uno::Any>> aOldProps; + // If there is a format, check if the shape already has a textbox assigned to. - if (auto pTextBoxNode = pShapeFormat->GetOtherTextBoxFormats()) + if (auto& pTextBoxNode = pShapeFormat->GetOtherTextBoxFormats()) { // If it has a texbox, destroy it. if (pTextBoxNode->GetTextBox(pObj)) - { - auto xOldFrame - = pObj->getUnoShape()->queryInterface(cppu::UnoType<text::XTextRange>::get()); - if (xOldFrame.hasValue()) - { - uno::Reference<beans::XPropertySet> xOldprops(xOldFrame, uno::UNO_QUERY); - uno::Reference<beans::XPropertyState> xOldPropStates(xOldFrame, uno::UNO_QUERY); - for (auto& rProp : xOldprops->getPropertySetInfo()->getProperties()) - { - try - { - if (xOldPropStates->getPropertyState(rProp.Name) - == beans::PropertyState::PropertyState_DIRECT_VALUE) - aOldProps.push_back( - std::pair(rProp, xOldprops->getPropertyValue(rProp.Name))); - } - catch (...) - { - } - } - } - destroy(pShapeFormat, pObj); - } + pTextBoxNode->DelTextBox(pObj, true); // And set the new one. pTextBoxNode->AddTextBox(pObj, pFormat); pFormat->SetOtherTextBoxFormats(pTextBoxNode); @@ -297,25 +272,8 @@ void SwTextBoxHelper::set(SwFrameFormat* pShapeFormat, SdrObject* pObj, xPropertySet->setPropertyValue(UNO_NAME_TEXT_VERT_ADJUST, uno::makeAny(aVertAdj)); text::WritingMode eMode; if (xShapePropertySet->getPropertyValue(UNO_NAME_TEXT_WRITINGMODE) >>= eMode) - syncProperty(pShapeFormat, RES_FRAMEDIR, 0, uno::makeAny(sal_Int16(eMode)), pObj); - if (aOldProps.size()) - { - for (auto& rProp : aOldProps) - { - try - { - xPropertySet->setPropertyValue(rProp.first.Name, rProp.second); - } - catch (...) - { - } - } - } - if (pFormat->GetAnchor().GetAnchorId() == RndStdIds::FLY_AT_PAGE - && pFormat->GetAnchor().GetPageNum() == 0) - { - pFormat->SetFormatAttr(SwFormatAnchor(RndStdIds::FLY_AT_PAGE, 1)); - } + syncProperty(pShapeFormat, RES_FRAMEDIR, 0, uno::Any(sal_Int16(eMode)), pObj); + // Do sync for the new textframe. synchronizeGroupTextBoxProperty(&changeAnchor, pShapeFormat, pObj); synchronizeGroupTextBoxProperty(&syncTextBoxSize, pShapeFormat, pObj); @@ -326,12 +284,10 @@ void SwTextBoxHelper::set(SwFrameFormat* pShapeFormat, SdrObject* pObj, void SwTextBoxHelper::destroy(const SwFrameFormat* pShape, const SdrObject* pObject) { // If a TextBox was enabled previously - auto pTextBox = pShape->GetOtherTextBoxFormats(); - if (pTextBox && pTextBox->IsTextBoxActive(pObject)) + auto& pTextBox = pShape->GetOtherTextBoxFormats(); + if (pTextBox) { // Unlink the TextBox's text range from the original shape. - pTextBox->SetTextBoxInactive(pObject); - // Delete the associated TextFrame. pTextBox->DelTextBox(pObject, true); } @@ -345,7 +301,7 @@ bool SwTextBoxHelper::isTextBox(const SwFrameFormat* pFormat, sal_uInt16 nType, if (!pFormat || pFormat->Which() != nType) return false; - auto pTextBox = pFormat->GetOtherTextBoxFormats(); + auto& pTextBox = pFormat->GetOtherTextBoxFormats(); if (!pTextBox) return false; @@ -988,7 +944,7 @@ void SwTextBoxHelper::syncProperty(SwFrameFormat* pShape, sal_uInt16 nWID, sal_u } } } - + auto aGuard = SwTextBoxLockGuard(*pShape->GetOtherTextBoxFormats()); uno::Reference<beans::XPropertySet> const xPropertySet( SwXTextFrame::CreateXTextFrame(*pFormat->GetDoc(), pFormat), uno::UNO_QUERY); xPropertySet->setPropertyValue(aPropertyName, aValue); @@ -1182,8 +1138,10 @@ void SwTextBoxHelper::syncFlyFrameAttr(SwFrameFormat& rShape, SfxItemSet const& } while (pItem && (0 != pItem->Which())); if (aTextBoxSet.Count()) + { + auto aGuard = SwTextBoxLockGuard(*rShape.GetOtherTextBoxFormats()); pFormat->SetFormatAttr(aTextBoxSet); - + } DoTextBoxZOrderCorrection(&rShape, pObj); } @@ -1235,6 +1193,22 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) { if (auto pFormat = getOtherTextBoxFormat(pShape, RES_DRAWFRMFMT, pObj)) { + if (!isAnchorSyncNeeded(pShape, pFormat)) + { + doTextBoxPositioning(pShape, pObj); + DoTextBoxZOrderCorrection(pShape, pObj); + if (pShape->GetAnchor().GetAnchorId() == RndStdIds::FLY_AS_CHAR + && pFormat->GetAnchor().GetAnchorId() == RndStdIds::FLY_AT_CHAR + && pFormat->GetVertOrient().GetRelationOrient() != text::RelOrientation::PRINT_AREA) + { + SwFormatVertOrient aTmp = pFormat->GetVertOrient(); + aTmp.SetRelationOrient(text::RelOrientation::PRINT_AREA); + pFormat->SetFormatAttr(aTmp); + } + + return false; + } + const SwFormatAnchor& rOldAnch = pFormat->GetAnchor(); const SwFormatAnchor& rNewAnch = pShape->GetAnchor(); @@ -1246,6 +1220,7 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) try { + auto aGuard = SwTextBoxLockGuard(*pShape->GetOtherTextBoxFormats()); ::sw::UndoGuard const UndoGuard(pShape->GetDoc()->GetIDocumentUndoRedo()); uno::Reference<beans::XPropertySet> const xPropertySet( SwXTextFrame::CreateXTextFrame(*pFormat->GetDoc(), pFormat), uno::UNO_QUERY); @@ -1262,6 +1237,7 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) { if (rNewAnch.GetAnchorId() == RndStdIds::FLY_AS_CHAR) { + assert(pNewCnt); uno::Any aValue(text::TextContentAnchorType_AT_CHARACTER); xPropertySet->setPropertyValue(UNO_NAME_ANCHOR_TYPE, aValue); xPropertySet->setPropertyValue(UNO_NAME_HORI_ORIENT_RELATION, @@ -1285,6 +1261,7 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) { if (rNewAnch.GetAnchorId() == RndStdIds::FLY_AS_CHAR) { + assert(pNewCnt); uno::Any aValue(text::TextContentAnchorType_AT_CHARACTER); xPropertySet->setPropertyValue(UNO_NAME_ANCHOR_TYPE, aValue); xPropertySet->setPropertyValue(UNO_NAME_HORI_ORIENT_RELATION, @@ -1299,7 +1276,13 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) { xPropertySet->setPropertyValue(UNO_NAME_HORI_ORIENT_RELATION, aShapeHorRelOrient); - pFormat->SetFormatAttr(pShape->GetAnchor()); + if (rNewAnch.GetAnchorId() == RndStdIds::FLY_AT_PAGE + && rNewAnch.GetPageNum() == 0) + { + pFormat->SetFormatAttr(SwFormatAnchor(RndStdIds::FLY_AT_PAGE, 1)); + } + else + pFormat->SetFormatAttr(pShape->GetAnchor()); } } } @@ -1308,7 +1291,9 @@ bool SwTextBoxHelper::changeAnchor(SwFrameFormat* pShape, SdrObject* pObj) SAL_WARN("sw.core", "SwTextBoxHelper::changeAnchor(): " << e.Message); } - return doTextBoxPositioning(pShape, pObj) && DoTextBoxZOrderCorrection(pShape, pObj); + doTextBoxPositioning(pShape, pObj); + DoTextBoxZOrderCorrection(pShape, pObj); + return true; } return false; @@ -1320,6 +1305,8 @@ bool SwTextBoxHelper::doTextBoxPositioning(SwFrameFormat* pShape, SdrObject* pOb if (auto pFormat = getOtherTextBoxFormat(pShape, RES_DRAWFRMFMT, pObj)) { ::sw::UndoGuard const UndoGuard(pShape->GetDoc()->GetIDocumentUndoRedo()); + auto aGuard = SwTextBoxLockGuard(*pShape->GetOtherTextBoxFormats()); + // Special treatment for AS_CHAR textboxes: if (pShape->GetAnchor().GetAnchorId() == RndStdIds::FLY_AS_CHAR) { tools::Rectangle aRect( @@ -1416,6 +1403,7 @@ bool SwTextBoxHelper::syncTextBoxSize(SwFrameFormat* pShape, SdrObject* pObj) if (auto pTextBox = getOtherTextBoxFormat(pShape, RES_DRAWFRMFMT, pObj)) { + auto aGuard = SwTextBoxLockGuard(*pShape->GetOtherTextBoxFormats()); const auto& rSize = getTextRectangle(pObj, false).GetSize(); if (!rSize.IsEmpty()) { @@ -1438,6 +1426,8 @@ bool SwTextBoxHelper::DoTextBoxZOrderCorrection(SwFrameFormat* pShape, const Sdr if (pShpObj) { auto pTextBox = getOtherTextBoxFormat(pShape, RES_DRAWFRMFMT, pObj); + if (!pTextBox) + return false; SdrObject* pFrmObj = pTextBox->FindRealSdrObject(); if (!pFrmObj) { @@ -1525,19 +1515,103 @@ std::vector<SwFrameFormat*> SwTextBoxHelper::CollectTextBoxes(SdrObject* pGroupO return vRet; } +bool SwTextBoxHelper::isAnchorSyncNeeded(const SwFrameFormat* pFirst, const SwFrameFormat* pSecond) +{ + if (!pFirst) + return false; + + if (!pSecond) + return false; + + if (pFirst == pSecond) + return false; + + if (!pFirst->GetOtherTextBoxFormats()) + return false; + + if (!pSecond->GetOtherTextBoxFormats()) + return false; + + if (pFirst->GetOtherTextBoxFormats() != pSecond->GetOtherTextBoxFormats()) + return false; + + if (pFirst->GetOtherTextBoxFormats()->GetOwnerShape() == pSecond + || pFirst == pSecond->GetOtherTextBoxFormats()->GetOwnerShape()) + { + const auto& rShapeAnchor + = pFirst->Which() == RES_DRAWFRMFMT ? pFirst->GetAnchor() : pSecond->GetAnchor(); + const auto& rFrameAnchor + = pFirst->Which() == RES_FLYFRMFMT ? pFirst->GetAnchor() : pSecond->GetAnchor(); + + if (rShapeAnchor.GetAnchorId() == rFrameAnchor.GetAnchorId()) + { + if (rShapeAnchor.GetContentAnchor() && rFrameAnchor.GetContentAnchor()) + { + if (rShapeAnchor.GetContentAnchor()->nContent + != rFrameAnchor.GetContentAnchor()->nContent) + return true; + + if (rShapeAnchor.GetContentAnchor()->nNode + != rFrameAnchor.GetContentAnchor()->nNode) + return true; + + return false; + } + + if (rShapeAnchor.GetAnchorId() == RndStdIds::FLY_AT_PAGE + && rFrameAnchor.GetAnchorId() == RndStdIds::FLY_AT_PAGE) + { + if (rShapeAnchor.GetPageNum() == rFrameAnchor.GetPageNum()) + return false; + else + return true; + } + + return true; + } + + if (rShapeAnchor.GetAnchorId() == RndStdIds::FLY_AS_CHAR + && rFrameAnchor.GetAnchorId() == RndStdIds::FLY_AT_CHAR) + { + if (rShapeAnchor.GetContentAnchor() && rFrameAnchor.GetContentAnchor()) + { + if (rShapeAnchor.GetContentAnchor()->nContent + != rFrameAnchor.GetContentAnchor()->nContent) + return true; + + if (rShapeAnchor.GetContentAnchor()->nNode + != rFrameAnchor.GetContentAnchor()->nNode) + return true; + + return false; + } + } + return true; + } + return false; +} + SwTextBoxNode::SwTextBoxNode(SwFrameFormat* pOwnerShape) { assert(pOwnerShape); assert(pOwnerShape->Which() == RES_DRAWFRMFMT); m_bIsCloningInProgress = false; + m_bLock = false; m_pOwnerShapeFormat = pOwnerShape; if (!m_pTextBoxes.empty()) m_pTextBoxes.clear(); } -SwTextBoxNode::~SwTextBoxNode() { m_pTextBoxes.clear(); } +SwTextBoxNode::~SwTextBoxNode() +{ + if (m_pTextBoxes.size() != 0) + { + SAL_WARN("sw.core", "SwTextBoxNode::~SwTextBoxNode(): Text-Box-Vector still not empty!"); + assert(false); + } +} void SwTextBoxNode::AddTextBox(SdrObject* pDrawObject, SwFrameFormat* pNewTextBox) { @@ -1547,9 +1621,18 @@ void SwTextBoxNode::AddTextBox(SdrObject* pDrawObject, SwFrameFormat* pNewTextBo assert(pDrawObject); SwTextBoxElement aElem; - aElem.m_bIsActive = true; aElem.m_pDrawObject = pDrawObject; aElem.m_pTextBoxFormat = pNewTextBox; + + for (const auto& rE : m_pTextBoxes) + { + if (rE.m_pDrawObject == pDrawObject || rE.m_pTextBoxFormat == pNewTextBox) + { + SAL_WARN("sw.core", "SwTextBoxNode::AddTextBox(): Already exist!"); + return; + } + } + auto pSwFlyDraw = dynamic_cast<SwFlyDrawObj*>(pDrawObject); if (pSwFlyDraw) { @@ -1570,20 +1653,22 @@ void SwTextBoxNode::DelTextBox(const SdrObject* pDrawObject, bool bDelFromDoc) { if (bDelFromDoc) { - m_pOwnerShapeFormat->GetDoc()->getIDocumentLayoutAccess().DelLayoutFormat( + it->m_pTextBoxFormat->GetDoc()->getIDocumentLayoutAccess().DelLayoutFormat( it->m_pTextBoxFormat); // What about m_pTextBoxes? So, when the DelLayoutFormat() removes the format // then the ~SwFrameFormat() will call this method again to remove the entry. - break; + return; } else { it = m_pTextBoxes.erase(it); - break; + return; } } ++it; } + + SAL_WARN("sw.core", "SwTextBoxNode::DelTextBox(): Not found!"); } void SwTextBoxNode::DelTextBox(const SwFrameFormat* pTextBox, bool bDelFromDoc) @@ -1597,41 +1682,40 @@ void SwTextBoxNode::DelTextBox(const SwFrameFormat* pTextBox, bool bDelFromDoc) { if (bDelFromDoc) { - m_pOwnerShapeFormat->GetDoc()->getIDocumentLayoutAccess().DelLayoutFormat( + it->m_pTextBoxFormat->GetDoc()->getIDocumentLayoutAccess().DelLayoutFormat( it->m_pTextBoxFormat); // What about m_pTextBoxes? So, when the DelLayoutFormat() removes the format // then the ~SwFrameFormat() will call this method again to remove the entry. - break; + return; } else { it = m_pTextBoxes.erase(it); - break; + return; } } ++it; } + + SAL_WARN("sw.core", "SwTextBoxNode::DelTextBox(): Not found!"); } SwFrameFormat* SwTextBoxNode::GetTextBox(const SdrObject* pDrawObject) const { assert(pDrawObject); - if (!m_pTextBoxes.empty()) + assert(m_pOwnerShapeFormat); + + if (auto& pTextBoxes = m_pOwnerShapeFormat->GetOtherTextBoxFormats()) { - for (auto it = m_pTextBoxes.begin(); it != m_pTextBoxes.end(); it++) + if (size_t(pTextBoxes.use_count()) != pTextBoxes->GetTextBoxCount() + size_t(1)) { - if (it->m_pDrawObject == pDrawObject) - { - return it->m_pTextBoxFormat; - } + SAL_WARN("sw.core", "SwTextBoxNode::GetTextBox(): RefCount and TexBox count mismatch!"); + assert(false); } } - return nullptr; -} -bool SwTextBoxNode::IsTextBoxActive(const SdrObject* pDrawObject) const -{ - assert(pDrawObject); + if (m_bLock) + return nullptr; if (!m_pTextBoxes.empty()) { @@ -1639,42 +1723,58 @@ bool SwTextBoxNode::IsTextBoxActive(const SdrObject* pDrawObject) const { if (it->m_pDrawObject == pDrawObject) { - return it->m_bIsActive; + return it->m_pTextBoxFormat; } } + SAL_WARN("sw.core", "SwTextBoxNode::GetTextBox(): Not found!"); } - return false; + + return nullptr; } -void SwTextBoxNode::SetTextBoxActive(const SdrObject* pDrawObject) +void SwTextBoxNode::ClearAll() { - assert(pDrawObject); + // If this called from ~SwDoc(), then only the address entries + // have to be removed, the format will be deleted by the + // the mpSpzFrameFormatTable->DeleteAndDestroyAll() in ~SwDoc()! + if (m_pOwnerShapeFormat->GetDoc()->IsInDtor()) + { + m_pTextBoxes.clear(); + return; + } - if (!m_pTextBoxes.empty()) + // For loop control + sal_uInt16 nLoopCount = 0; + + // Reference not enough, copy needed. + const size_t nTextBoxCount = m_pTextBoxes.size(); + + // For loop has problems: When one entry deleted, the iterator has + // to be refreshed according to the new situation. So using While() instead. + while (!m_pTextBoxes.empty()) { - for (auto it = m_pTextBoxes.begin(); it != m_pTextBoxes.end(); it++) + // Delete the last textbox of the vector from the doc + // (what will call deregister in ~SwFrameFormat() + m_pOwnerShapeFormat->GetDoc()->getIDocumentLayoutAccess().DelLayoutFormat( + m_pTextBoxes.back().m_pTextBoxFormat); + + // Check if we are looping + if (nLoopCount > (nTextBoxCount + 1)) { - if (it->m_pDrawObject == pDrawObject) - { - it->m_bIsActive = true; - } + SAL_WARN("sw.core", "SwTextBoxNode::ClearAll(): Maximum loop count reached!"); + break; + } + else + { + nLoopCount++; } } -} - -void SwTextBoxNode::SetTextBoxInactive(const SdrObject* pDrawObject) -{ - assert(pDrawObject); + // Ensure the vector is empty. if (!m_pTextBoxes.empty()) { - for (auto it = m_pTextBoxes.begin(); it != m_pTextBoxes.end(); it++) - { - if (it->m_pDrawObject == pDrawObject) - { - it->m_bIsActive = false; - } - } + SAL_WARN("sw.core", "SwTextBoxNode::ClearAll(): Text-Box-Vector still not empty!"); + assert(false); } } @@ -1708,6 +1808,14 @@ void SwTextBoxNode::Clone(SwDoc* pDoc, const SwFormatAnchor& rNewAnc, SwFrameFor o_pTarget->FindSdrObject(), bSetAttr, bMakeFrame); m_bIsCloningInProgress = false; + + for (auto& rElem : m_pTextBoxes) + { + SwTextBoxHelper::changeAnchor(m_pOwnerShapeFormat, rElem.m_pDrawObject); + SwTextBoxHelper::doTextBoxPositioning(m_pOwnerShapeFormat, rElem.m_pDrawObject); + SwTextBoxHelper::DoTextBoxZOrderCorrection(m_pOwnerShapeFormat, rElem.m_pDrawObject); + SwTextBoxHelper::syncTextBoxSize(m_pOwnerShapeFormat, rElem.m_pDrawObject); + } } void SwTextBoxNode::Clone_Impl(SwDoc* pDoc, const SwFormatAnchor& rNewAnc, SwFrameFormat* o_pTarget, @@ -1723,7 +1831,10 @@ void SwTextBoxNode::Clone_Impl(SwDoc* pDoc, const SwFormatAnchor& rNewAnc, SwFra if (pSrcList && pDestList) { if (pSrcList->GetObjCount() != pDestList->GetObjCount()) + { + SAL_WARN("sw.core", "SwTextBoxNode::Clone_Impl(): Difference between the shapes!"); return; + } for (size_t i = 0; i < pSrcList->GetObjCount(); ++i) { diff --git a/sw/source/core/edit/edundo.cxx b/sw/source/core/edit/edundo.cxx index 826d3154ff78..ec2a365e2662 100644 --- a/sw/source/core/edit/edundo.cxx +++ b/sw/source/core/edit/edundo.cxx @@ -34,6 +34,8 @@ #include <frmfmt.hxx> #include <docsh.hxx> #include <pagefrm.hxx> +#include <textboxhelper.hxx> +#include <fmtanchr.hxx> #include <wrtsh.hxx> @@ -64,6 +66,13 @@ void SwEditShell::HandleUndoRedoContext(::sw::UndoRedoContext & rContext) if (RES_DRAWFRMFMT == pSelFormat->Which()) { SdrObject* pSObj = pSelFormat->FindSdrObject(); + + // Before layout calc, inline anchored textboxes have to be synced unless crash. + if (pSelFormat->GetAnchor().GetAnchorId() == RndStdIds::FLY_AS_CHAR + && pSelFormat->GetOtherTextBoxFormats() && pSObj) + SwTextBoxHelper::synchronizeGroupTextBoxProperty(SwTextBoxHelper::changeAnchor, + pSelFormat, pSObj); + static_cast<SwFEShell*>(this)->SelectObj( pSObj->GetCurrentBoundRect().Center() ); } diff --git a/sw/source/core/layout/atrfrm.cxx b/sw/source/core/layout/atrfrm.cxx index 3734963ead87..ebe0033ee5dc 100644 --- a/sw/source/core/layout/atrfrm.cxx +++ b/sw/source/core/layout/atrfrm.cxx @@ -2547,14 +2547,23 @@ SwFrameFormat::~SwFrameFormat() } } + // Check if there any textboxes attached to this format. if( nullptr == m_pOtherTextBoxFormats ) return; // This is a fly-frame-format just delete this // textbox entry from the textbox collection. + // Note: Do not delete it from the doc, as that + // is already in progress. if (Which() == RES_FLYFRMFMT) m_pOtherTextBoxFormats->DelTextBox(this); + // This is a draw-frame-format what belongs to + // a shape with textbox(es). Delete all of them. + if (Which() == RES_DRAWFRMFMT) + m_pOtherTextBoxFormats->ClearAll(); + + // Release the pointer. m_pOtherTextBoxFormats.reset(); } diff --git a/sw/source/core/text/porfly.cxx b/sw/source/core/text/porfly.cxx index 8944abe1994d..221827cc2310 100644 --- a/sw/source/core/text/porfly.cxx +++ b/sw/source/core/text/porfly.cxx @@ -366,12 +366,17 @@ void SwFlyCntPortion::SetBase( const SwTextFrame& rFrame, const Point &rBase, if (auto pFormat = FindFrameFormat(pSdrObj)) { - if (pFormat->GetOtherTextBoxFormats()) + if (pFormat->GetOtherTextBoxFormats() + && pFormat->GetAnchor().GetAnchorId() == RndStdIds::FLY_AS_CHAR) { + // TODO: Improve security with moving this sync call to other place, + // where it works for typing but not during layout calc. const bool bModified = pFormat->GetDoc()->getIDocumentState().IsEnableSetModified(); pFormat->GetDoc()->getIDocumentState().SetEnableSetModified(false); SwTextBoxHelper::synchronizeGroupTextBoxProperty(SwTextBoxHelper::changeAnchor, pFormat, pFormat->FindRealSdrObject()); + SwTextBoxHelper::synchronizeGroupTextBoxProperty(SwTextBoxHelper::syncTextBoxSize, + pFormat, pFormat->FindRealSdrObject()); pFormat->GetDoc()->getIDocumentState().SetEnableSetModified(bModified); } } diff --git a/sw/source/core/undo/undobj1.cxx b/sw/source/core/undo/undobj1.cxx index fb624d68d6b0..2b989872f93a 100644 --- a/sw/source/core/undo/undobj1.cxx +++ b/sw/source/core/undo/undobj1.cxx @@ -56,8 +56,16 @@ SwUndoFlyBase::~SwUndoFlyBase() { if( m_bDelFormat ) // delete during an Undo? { - if (m_pFrameFormat->GetOtherTextBoxFormats()) - { // clear that before delete + if (const auto& pTextBoxes = m_pFrameFormat->GetOtherTextBoxFormats()) + { + // Clear and unregister before release. + if (m_pFrameFormat->Which() == RES_FLYFRMFMT) + pTextBoxes->DelTextBox(m_pFrameFormat); + + if (m_pFrameFormat->Which() == RES_DRAWFRMFMT) + pTextBoxes->ClearAll(); + + // clear that before delete m_pFrameFormat->SetOtherTextBoxFormats(nullptr); } delete m_pFrameFormat; diff --git a/sw/source/core/unocore/unodraw.cxx b/sw/source/core/unocore/unodraw.cxx index 1104df24085a..18b7be2fc29d 100644 --- a/sw/source/core/unocore/unodraw.cxx +++ b/sw/source/core/unocore/unodraw.cxx @@ -1164,19 +1164,22 @@ void SwXShape::setPropertyValue(const OUString& rPropertyName, const uno::Any& a } else if (pEntry->nWID == FN_TEXT_BOX) { + auto pObj = SdrObject::getSdrObjectFromXShape(mxShape); if (pEntry->nMemberId == MID_TEXT_BOX) { bool bValue(false); aValue >>= bValue; + if (bValue) - SwTextBoxHelper::create(pFormat, GetSvxShape()->GetSdrObject()); + SwTextBoxHelper::create(pFormat, pObj); else - SwTextBoxHelper::destroy(pFormat, GetSvxShape()->GetSdrObject()); + SwTextBoxHelper::destroy(pFormat, pObj); } else if (pEntry->nMemberId == MID_TEXT_BOX_CONTENT) { - if (aValue.getValueType() == cppu::UnoType<uno::Reference<text::XTextFrame>>::get()) - SwTextBoxHelper::set(pFormat, GetSvxShape()->GetSdrObject(), + if (aValue.getValueType() + == cppu::UnoType<uno::Reference<text::XTextFrame>>::get()) + SwTextBoxHelper::set(pFormat, pObj, aValue.get<uno::Reference<text::XTextFrame>>()); else SAL_WARN( "sw.uno", "This is not a TextFrame!" ); @@ -1352,10 +1355,21 @@ void SwXShape::setPropertyValue(const OUString& rPropertyName, const uno::Any& a } if( bSetAttr ) pFormat->SetFormatAttr(aSet); + + // If this property is an anchor change, and there is a group shape with textboxes + // do anchor sync in time unless the anchor sync in the porfly will cause crash during + // layout calculation (When importing an inline shape in docx via dmapper). + if (pFormat->Which() == RES_DRAWFRMFMT && pFormat->GetOtherTextBoxFormats() + && pFormat->GetOtherTextBoxFormats()->GetTextBoxCount() + > o3tl::make_unsigned(1)) + SwTextBoxHelper::synchronizeGroupTextBoxProperty( + SwTextBoxHelper::changeAnchor, pFormat, + SdrObject::getSdrObjectFromXShape(mxShape)); } else pFormat->SetFormatAttr(aSet); } + // We have a pFormat and a pEntry as well: try to sync TextBox property. SwTextBoxHelper::syncProperty(pFormat, pEntry->nWID, pEntry->nMemberId, aValue, SdrObject::getSdrObjectFromXShape(mxShape)); diff --git a/sw/source/core/unocore/unotext.cxx b/sw/source/core/unocore/unotext.cxx index 9add198df938..8d91dfe636d1 100644 --- a/sw/source/core/unocore/unotext.cxx +++ b/sw/source/core/unocore/unotext.cxx @@ -1771,6 +1771,29 @@ SwXText::convertToTextFrame( aAnchor.SetAnchor(aMovePam.Start()); m_pImpl->m_pDoc->SetAttr(aAnchor, *pFrameFormat); } + else + { + // if this frame is a textbox of a shape anchored to us, move this textbox too. + const auto& pTextBoxes = pFrameFormat->GetOtherTextBoxFormats(); + if (pFrameFormat->Which() == RES_FLYFRMFMT && pTextBoxes + && pTextBoxes->GetOwnerShape()) + { + const auto& rShapeAnchor = pTextBoxes->GetOwnerShape()->GetAnchor(); + if (rShapeAnchor.GetAnchorId() == RndStdIds::FLY_AS_CHAR + && rShapeAnchor.GetContentAnchor() && pFrameFormat->GetAnchor().GetContentAnchor() + && pStartPam->ContainsPosition(*pFrameFormat->GetAnchor().GetContentAnchor())) + { + const auto& rAnchorNode + = pFrameFormat->GetAnchor().GetContentAnchor()->nNode.GetNode(); + if (!(rAnchorNode.FindFooterStartNode() || rAnchorNode.FindHeaderStartNode())) + { + SwFormatAnchor aAnchor(pFrameFormat->GetAnchor()); + aAnchor.SetAnchor(aMovePam.Start()); + m_pImpl->m_pDoc->SetAttr(aAnchor, *pFrameFormat); + } + } + } + } } } } diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 5fb0410cd84a..e176e38c945d 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -593,8 +593,8 @@ void DomainMapper_Impl::RemoveDummyParaForTableInSection() } void DomainMapper_Impl::AddDummyParaForTableInSection() { - // Shapes can't have sections. - if (IsInShape()) + // Shapes and textboxes can't have sections. + if (IsInShape() || m_bIsInTextBox) return; if (!m_aTextAppendStack.empty()) @@ -3772,9 +3772,13 @@ void DomainMapper_Impl::PushShapeContext( const uno::Reference< drawing::XShape { try { - uno::Reference<text::XTextRange> xFrame(xShapes->getByIndex(i), uno::UNO_QUERY_THROW); - uno::Reference<beans::XPropertySet> xSyncedPropertySet(xFrame, uno::UNO_QUERY_THROW); - comphelper::SequenceAsHashMap aGrabBag( xSyncedPropertySet->getPropertyValue("CharInteropGrabBag") ); + uno::Reference<text::XTextRange> xFrame(xShapes->getByIndex(i), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xFramePropertySet; + if (xFrame) + xFramePropertySet.set(xFrame, uno::UNO_QUERY_THROW); + uno::Reference<beans::XPropertySet> xShapePropertySet(xShapes->getByIndex(i), uno::UNO_QUERY_THROW); + + comphelper::SequenceAsHashMap aGrabBag( xShapePropertySet->getPropertyValue("CharInteropGrabBag") ); // only VML import has checked for style. Don't apply default parastyle properties to other imported shapes // - except for fontsize - to maintain compatibility with previous versions of LibreOffice. @@ -3802,7 +3806,7 @@ void DomainMapper_Impl::PushShapeContext( const uno::Reference< drawing::XShape PROP_CHAR_COLOR, PROP_PARA_ADJUST }; - const uno::Reference<beans::XPropertyState> xSyncedPropertyState(xSyncedPropertySet, uno::UNO_QUERY_THROW); + const uno::Reference<beans::XPropertyState> xShapePropertyState(xShapePropertySet, uno::UNO_QUERY_THROW); for ( const auto& eId : eIds ) { try @@ -3811,11 +3815,16 @@ void DomainMapper_Impl::PushShapeContext( const uno::Reference< drawing::XShape continue; const OUString sPropName = getPropertyName(eId); - if ( beans::PropertyState_DEFAULT_VALUE == xSyncedPropertyState->getPropertyState(sPropName) ) + if ( beans::PropertyState_DEFAULT_VALUE == xShapePropertyState->getPropertyState(sPropName) ) { const uno::Any aProp = GetPropertyFromStyleSheet(eId, pEntry, /*bDocDefaults=*/true, /*bPara=*/true); - if ( aProp.hasValue() ) - xSyncedPropertySet->setPropertyValue( sPropName, aProp ); + if (aProp.hasValue()) + { + if (xFrame) + xFramePropertySet->setPropertyValue(sPropName, aProp); + else + xShapePropertySet->setPropertyValue(sPropName, aProp); + } } } catch (const uno::Exception&)