sw/qa/extras/ooxmlexport/data/tdf127622_framePr.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 6 writerfilter/source/dmapper/DomainMapper_Impl.cxx | 126 ++++++++++--------- writerfilter/source/dmapper/DomainMapper_Impl.hxx | 1 writerfilter/source/dmapper/PropertyMap.cxx | 21 --- writerfilter/source/dmapper/PropertyMap.hxx | 3 6 files changed, 75 insertions(+), 82 deletions(-)
New commits: commit e0e33aa622968bc2e182099670a024a1d00b8e5c Author: Justin Luth <[email protected]> AuthorDate: Tue Apr 4 14:20:45 2023 -0400 Commit: Miklos Vajna <[email protected]> CommitDate: Wed Apr 12 09:14:33 2023 +0200 tdf#105035 tdf#127622 writerfilter framePr: compare using style info Prior to this patch, we were only comparing frame definitions based on direct formatting properties. Well, the definition can be spread throughout the paragraph styles, and only if the end result matches will they all be part of one frame. This patch fixes bug 127622 where three different styles all had the same framePr settings. It partially fixes bug 105035 - the frame is split in two - but now the frames overlap. There is no "overlap" tick box in MSO, although they do allow overlap - just not from other framePr text. So this could end up giving me a lot of grief, where two frames joined look OK, but split they will almost inevitably be stuck overtop of each other - like we previously saw in bug 127622. make CppunitTest_sw_ooxmlexport18 CPPUNIT_TEST_NAME=testTdf127622_framePr Change-Id: Ie9ce9076d06029b8789a95b46f4ed49190ab09f7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150036 Tested-by: Jenkins Reviewed-by: Justin Luth <[email protected]> diff --git a/sw/qa/extras/ooxmlexport/data/tdf127622_framePr.docx b/sw/qa/extras/ooxmlexport/data/tdf127622_framePr.docx new file mode 100644 index 000000000000..5440a81985fc Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf127622_framePr.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx index 6f84d9784250..caf312ba9198 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx @@ -146,6 +146,12 @@ DECLARE_OOXMLEXPORT_TEST(testTdf146984_anchorInShape, "tdf146984_anchorInShape.d assertXPath(pLayout, "//page[2]//anchored", 2); } +DECLARE_OOXMLEXPORT_TEST(testTdf127622_framePr, "tdf127622_framePr.docx") +{ + // All the paragraphs end up with the same frame definition, so put them all in one frame + CPPUNIT_ASSERT_EQUAL(1, getShapes()); +} + DECLARE_OOXMLEXPORT_TEST(testTdf154129_framePr1, "tdf154129_framePr1.docx") { for (size_t i = 1; i < 4; ++i) diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index 8511417e852c..11247cbf6b77 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -1578,10 +1578,9 @@ static void lcl_MoveBorderPropertiesToFrame(std::vector<beans::PropertyValue>& r } -static void lcl_AddRangeAndStyle( +static void lcl_AddRange( ParagraphPropertiesPtr const & pToBeSavedProperties, uno::Reference< text::XTextAppend > const& xTextAppend, - const PropertyMapPtr& pPropertyMap, TextAppendContext const & rAppendContext) { uno::Reference<text::XParagraphCursor> xParaCursor( @@ -1590,16 +1589,6 @@ static void lcl_AddRangeAndStyle( xParaCursor->gotoStartOfParagraph( false ); pToBeSavedProperties->SetStartingRange(xParaCursor->getStart()); - if(pPropertyMap) - { - std::optional<PropertyMap::Property> aParaStyle = pPropertyMap->getProperty(PROP_PARA_STYLE_NAME); - if( aParaStyle ) - { - OUString sName; - aParaStyle->second >>= sName; - pToBeSavedProperties->SetParaStyleName(sName); - } - } } @@ -1608,28 +1597,19 @@ constexpr sal_Int32 DEFAULT_FRAME_MIN_WIDTH = 0; constexpr sal_Int32 DEFAULT_FRAME_MIN_HEIGHT = 0; constexpr sal_Int32 DEFAULT_VALUE = 0; -void DomainMapper_Impl::CheckUnregisteredFrameConversion( ) +std::vector<css::beans::PropertyValue> +DomainMapper_Impl::MakeFrameProperties(const ParagraphProperties& rProps) { - if (m_aTextAppendStack.empty()) - return; - TextAppendContext& rAppendContext = m_aTextAppendStack.top(); - // n#779642: ignore fly frame inside table as it could lead to messy situations - if (!rAppendContext.pLastParagraphProperties) - return; - if (!rAppendContext.pLastParagraphProperties->IsFrameMode()) - return; - if (!hasTableManager()) - return; - if (getTableManager().isInTable()) - return; + std::vector<beans::PropertyValue> aFrameProperties; + try { // A paragraph's properties come from direct formatting or somewhere in the style hierarchy std::vector<const ParagraphProperties*> vProps; - vProps.emplace_back(rAppendContext.pLastParagraphProperties.get()); + vProps.emplace_back(&rProps); sal_Int8 nSafetyLimit = 16; - StyleSheetEntryPtr pStyle = GetStyleSheetTable()->FindStyleSheetByConvertedStyleName( - rAppendContext.pLastParagraphProperties->GetParaStyleName()); + StyleSheetEntryPtr pStyle + = GetStyleSheetTable()->FindStyleSheetByConvertedStyleName(rProps.GetParaStyleName()); while (nSafetyLimit-- && pStyle && pStyle->m_pProperties) { vProps.emplace_back(&pStyle->m_pProperties->props()); @@ -1638,9 +1618,8 @@ void DomainMapper_Impl::CheckUnregisteredFrameConversion( ) break; pStyle = GetStyleSheetTable()->FindStyleSheetByISTD(pStyle->m_sBaseStyleIdentifier); } - SAL_WARN_IF(!nSafetyLimit,"writerfilter.dmapper","Inherited style loop likely: early exit"); + SAL_WARN_IF(!nSafetyLimit, "writerfilter.dmapper", "Inheritance loop likely: early exit"); - std::vector<beans::PropertyValue> aFrameProperties; sal_Int32 nWidth = -1; for (const auto pProp : vProps) @@ -1816,37 +1795,56 @@ void DomainMapper_Impl::CheckUnregisteredFrameConversion( ) aFrameProperties.push_back(comphelper::makePropertyValue( getPropertyName(PROP_BOTTOM_MARGIN), nVertOrient == text::VertOrientation::BOTTOM ? 0 : nBottomDist)); + } + catch (const uno::Exception&) + { + } - if (const std::optional<sal_Int16> nDirection = PopFrameDirection()) - { - aFrameProperties.push_back( - comphelper::makePropertyValue(getPropertyName(PROP_FRM_DIRECTION), *nDirection)); - } - - // If there is no fill, the Word default is 100% transparency. - // Otherwise CellColorHandler has priority, and this setting - // will be ignored. - aFrameProperties.push_back(comphelper::makePropertyValue( - getPropertyName(PROP_BACK_COLOR_TRANSPARENCY), sal_Int32(100))); + return aFrameProperties; +} - uno::Sequence<beans::PropertyValue> aGrabBag(comphelper::InitPropertySequence( - { { "ParaFrameProperties", - uno::Any(rAppendContext.pLastParagraphProperties->IsFrameMode()) } })); - aFrameProperties.push_back(comphelper::makePropertyValue("FrameInteropGrabBag", aGrabBag)); +void DomainMapper_Impl::CheckUnregisteredFrameConversion() +{ + if (m_aTextAppendStack.empty()) + return; + TextAppendContext& rAppendContext = m_aTextAppendStack.top(); + // n#779642: ignore fly frame inside table as it could lead to messy situations + if (!rAppendContext.pLastParagraphProperties) + return; + if (!rAppendContext.pLastParagraphProperties->IsFrameMode()) + return; + if (!hasTableManager()) + return; + if (getTableManager().isInTable()) + return; - lcl_MoveBorderPropertiesToFrame(aFrameProperties, - rAppendContext.pLastParagraphProperties->GetStartingRange(), - rAppendContext.pLastParagraphProperties->GetEndingRange()); + std::vector<beans::PropertyValue> aFrameProperties + = MakeFrameProperties(*rAppendContext.pLastParagraphProperties); - //frame conversion has to be executed after table conversion - RegisterFrameConversion( - rAppendContext.pLastParagraphProperties->GetStartingRange(), - rAppendContext.pLastParagraphProperties->GetEndingRange(), - std::move(aFrameProperties) ); - } - catch( const uno::Exception& ) + if (const std::optional<sal_Int16> nDirection = PopFrameDirection()) { + aFrameProperties.push_back( + comphelper::makePropertyValue(getPropertyName(PROP_FRM_DIRECTION), *nDirection)); } + + // If there is no fill, the Word default is 100% transparency. + // Otherwise CellColorHandler has priority, and this setting + // will be ignored. + aFrameProperties.push_back(comphelper::makePropertyValue( + getPropertyName(PROP_BACK_COLOR_TRANSPARENCY), sal_Int32(100))); + + uno::Sequence<beans::PropertyValue> aGrabBag(comphelper::InitPropertySequence( + { { "ParaFrameProperties", uno::Any(true) } })); + aFrameProperties.push_back(comphelper::makePropertyValue("FrameInteropGrabBag", aGrabBag)); + + lcl_MoveBorderPropertiesToFrame(aFrameProperties, + rAppendContext.pLastParagraphProperties->GetStartingRange(), + rAppendContext.pLastParagraphProperties->GetEndingRange()); + + //frame conversion has to be executed after table conversion, not now + RegisterFrameConversion(rAppendContext.pLastParagraphProperties->GetStartingRange(), + rAppendContext.pLastParagraphProperties->GetEndingRange(), + std::move(aFrameProperties)); } /// Check if the style or its parent has a list id, recursively. @@ -2219,6 +2217,16 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con old _and_ new DropCap must not occur */ + // The paragraph style is vital to knowing all the frame properties. + std::optional<PropertyMap::Property> aParaStyle + = pPropertyMap->getProperty(PROP_PARA_STYLE_NAME); + if (aParaStyle) + { + OUString sName; + aParaStyle->second >>= sName; + pParaContext->props().SetParaStyleName(sName); + } + bool bIsDropCap = pParaContext->props().IsFrameMode() && sal::static_int_cast<Id>(pParaContext->props().GetDropCap()) != NS_ooxml::LN_Value_doc_ST_DropCap_none; @@ -2255,7 +2263,9 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con if( pParaContext->props().IsFrameMode() ) pToBeSavedProperties = new ParagraphProperties(pParaContext->props()); } - else if(*rAppendContext.pLastParagraphProperties == pParaContext->props() ) + else if (pParaContext->props().IsFrameMode() + && MakeFrameProperties(*rAppendContext.pLastParagraphProperties) + == MakeFrameProperties(pParaContext->props())) { //handles (7) rAppendContext.pLastParagraphProperties->SetEndingRange(rAppendContext.xInsertPosition.is() ? rAppendContext.xInsertPosition : xTextAppend->getEnd()); @@ -2270,7 +2280,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con if ( !bIsDropCap && pParaContext->props().IsFrameMode() ) { pToBeSavedProperties = new ParagraphProperties(pParaContext->props()); - lcl_AddRangeAndStyle(pToBeSavedProperties, xTextAppend, pPropertyMap, rAppendContext); + lcl_AddRange(pToBeSavedProperties, xTextAppend, rAppendContext); } } } @@ -2281,7 +2291,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con if( !bIsDropCap && pParaContext->props().IsFrameMode() ) { pToBeSavedProperties = new ParagraphProperties(pParaContext->props()); - lcl_AddRangeAndStyle(pToBeSavedProperties, xTextAppend, pPropertyMap, rAppendContext); + lcl_AddRange(pToBeSavedProperties, xTextAppend, rAppendContext); } } std::vector<beans::PropertyValue> aProperties; diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx index 4731a8e31375..5d935b5e3664 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx @@ -1042,6 +1042,7 @@ public: bool IsInComments() const { return m_bIsInComments; }; + std::vector<css::beans::PropertyValue> MakeFrameProperties(const ParagraphProperties& rProps); void CheckUnregisteredFrameConversion( ); void RegisterFrameConversion(css::uno::Reference<css::text::XTextRange> const& xFrameStartRange, diff --git a/writerfilter/source/dmapper/PropertyMap.cxx b/writerfilter/source/dmapper/PropertyMap.cxx index 40d9eec86b7f..7794249350e8 100644 --- a/writerfilter/source/dmapper/PropertyMap.cxx +++ b/writerfilter/source/dmapper/PropertyMap.cxx @@ -2128,27 +2128,6 @@ ParagraphProperties::ParagraphProperties() { } -bool ParagraphProperties::operator==( const ParagraphProperties& rCompare ) const -{ - return ( m_bFrameMode == rCompare.m_bFrameMode && - m_nDropCap == rCompare.m_nDropCap && - m_nLines == rCompare.m_nLines && - m_w == rCompare.m_w && - m_h == rCompare.m_h && - m_nWrap == rCompare.m_nWrap && - m_hAnchor == rCompare.m_hAnchor && - m_vAnchor == rCompare.m_vAnchor && - m_x == rCompare.m_x && - m_bxValid == rCompare.m_bxValid && - m_y == rCompare.m_y && - m_byValid == rCompare.m_byValid && - m_hSpace == rCompare.m_hSpace && - m_vSpace == rCompare.m_vSpace && - m_hRule == rCompare.m_hRule && - m_xAlign == rCompare.m_xAlign && - m_yAlign == rCompare.m_yAlign ); -} - void ParagraphProperties::ResetFrameProperties() { m_bFrameMode = false; diff --git a/writerfilter/source/dmapper/PropertyMap.hxx b/writerfilter/source/dmapper/PropertyMap.hxx index 23c25bec3bb2..b458b70967b1 100644 --- a/writerfilter/source/dmapper/PropertyMap.hxx +++ b/writerfilter/source/dmapper/PropertyMap.hxx @@ -453,9 +453,6 @@ public: ParagraphProperties & operator =(ParagraphProperties const &) = default; ParagraphProperties & operator =(ParagraphProperties &&) = default; - // Does not compare the starting/ending range, m_sParaStyleName and m_nDropCapLength - bool operator==( const ParagraphProperties& ) const; - sal_Int32 GetListId() const { return m_nListId; } void SetListId( sal_Int32 nId ) { m_nListId = nId; }
