From f09420fa189be5165b0311083ba127073500a121 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Mon, 25 Dec 2023 15:32:41 +0600 Subject: tdf#158855: Make sure to not add extra paragraph after a table in a section Simplify the condition for last paragraph removal in lcl_utext, and clean up the complications piled after commit 2e8aad6d45c53d554ccaf26de998ede708cfc289 (fdo#39056 fdo#75431 Section Properties if section starts with table, 2014-04-24). This makes sure that the trailing extra paragraph gets properly removed, even when there was a dummy paragraph added for a table in section. Because of the removed check, import used to avoid the removal of the extra paragraph immediately following a table (when the table needed to be the last element in the section); hence, in DomainMapper_Impl::appendTextSectionAfter, a last section's table was followed by two paragraphs (one that is expected to stay after the created section, and one before that, that was not removed after the table). xCursor had ininitally both paragraphs in the selection after the forward goto* calls; and then, gotoLeft excluded only the last paragraph from the selection. After the change, in such a case, there is only one paragraph in the end: the one that should stay after the created section. But calling gotoLeft then would have to put the end of selection to a text node inside the table; since the selection that starts before the table can't end inside the table, the call resulted in the whole table excluded from the selection. This requires to add a paragraph here, to allow the previous behavior of cursor; and then the last paragraph in section is destroyed. The code makes sure that the trailing paragraph kept after the creation of the section has all the properties applied to the old last paragraph. testFdo53985 used to check for total 9 paragraphs in the document; while 3 of those were introduced exactly by the bug fixed here; they are absent in Word. The test is fixed; its logic is unchanged, and it still correctly tests that a round-trip doesn't increase number of paragraphs. Change-Id: Ibf6349ba7906dac185c759eb3b58c1849ee5064a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161275 Tested-by: Jenkins Reviewed-by: Mike Kaganski --- writerfilter/source/dmapper/DomainMapper_Impl.cxx | 137 ++++++++++++++++------ 1 file changed, 102 insertions(+), 35 deletions(-) (limited to 'writerfilter/source/dmapper/DomainMapper_Impl.cxx') diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx index bc6a3dfdf5a1..e247294a4e52 100644 --- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx +++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx @@ -29,6 +29,7 @@ #include "TagLogger.hxx" #include #include +#include #include #include #include @@ -380,7 +381,6 @@ DomainMapper_Impl::DomainMapper_Impl( m_bIsFirstParaInSection( true ), m_bIsFirstParaInSectionAfterRedline( true ), m_bDummyParaAddedForTableInSection( false ), - m_bDummyParaAddedForTableInSectionPage( false ), m_bTextFrameInserted(false), m_bIsPreviousParagraphFramed( false ), m_bIsLastParaInSection( false ), @@ -979,12 +979,6 @@ void DomainMapper_Impl::SetIsFirstParagraphInShape(bool bIsFirst) void DomainMapper_Impl::SetIsDummyParaAddedForTableInSection( bool bIsAdded ) { m_bDummyParaAddedForTableInSection = bIsAdded; - m_bDummyParaAddedForTableInSectionPage = bIsAdded; -} - -void DomainMapper_Impl::SetIsDummyParaAddedForTableInSectionPage( bool bIsAdded ) -{ - m_bDummyParaAddedForTableInSectionPage = bIsAdded; } @@ -3518,6 +3512,86 @@ void DomainMapper_Impl::adjustLastPara(sal_Int8 nAlign) pLastPara->Insert(PROP_PARA_ADJUST, uno::Any(nAlign), true); } +static void checkAndAddPropVal(const OUString& prop, const css::uno::Any& val, + std::vector& props, std::vector& values) +{ + // Avoid well-known reasons for exceptions when setting property values + if (!val.hasValue()) + return; + if (prop == "CharAutoStyleName" || prop == "ParaAutoStyleName") + return; + if (prop == "CharStyleName" || prop == "DropCapCharStyleName") + if (OUString val_string; (val >>= val_string) && val_string.isEmpty()) + return; + + props.push_back(prop); + values.push_back(val); +} + +static void copyAllProps(const css::uno::Reference& from, + const css::uno::Reference& to) +{ + css::uno::Reference xFromProps(from, css::uno::UNO_QUERY_THROW); + css::uno::Reference xFromInfo(xFromProps->getPropertySetInfo(), + css::uno::UNO_SET_THROW); + css::uno::Sequence rawProps(xFromInfo->getProperties()); + std::vector props; + props.reserve(rawProps.getLength()); + for (const auto& prop : rawProps) + if ((prop.Attributes & css::beans::PropertyAttribute::READONLY) == 0) + props.push_back(prop.Name); + std::vector values; + values.reserve(props.size()); + if (css::uno::Reference xFromMulti{ xFromProps, + css::uno::UNO_QUERY }) + { + const auto propsSeq = comphelper::containerToSequence(props); + const auto valuesSeq = xFromMulti->getPropertyValues(propsSeq); + assert(propsSeq.getLength() == valuesSeq.getLength()); + props.clear(); + for (size_t i = 0; i < propsSeq.size(); ++i) + checkAndAddPropVal(propsSeq[i], valuesSeq[i], props, values); + } + else + { + std::vector filtered_props; + filtered_props.reserve(props.size()); + for (const auto& prop : props) + checkAndAddPropVal(prop, xFromProps->getPropertyValue(prop), filtered_props, values); + filtered_props.swap(props); + } + assert(props.size() == values.size()); + + css::uno::Reference xToProps(to, css::uno::UNO_QUERY_THROW); + if (css::uno::Reference xToMulti{ xToProps, + css::uno::UNO_QUERY }) + { + try + { + xToMulti->setPropertyValues(comphelper::containerToSequence(props), + comphelper::containerToSequence(values)); + return; + } + catch (css::uno::Exception&) + { + DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper"); + } + // Fallback to property-by-property iteration + } + + for (size_t i = 0; i < props.size(); ++i) + { + try + { + xToProps->setPropertyValue(props[i], values[i]); + } + catch (css::uno::Exception&) + { + DBG_UNHANDLED_EXCEPTION("writerfilter.dmapper"); + } + } +} + uno::Reference< beans::XPropertySet > DomainMapper_Impl::appendTextSectionAfter( uno::Reference< text::XTextRange > const & xBefore ) { @@ -3537,40 +3611,33 @@ uno::Reference< beans::XPropertySet > DomainMapper_Impl::appendTextSectionAfter( xCursor->gotoRange( m_aTextAppendStack.top().xInsertPosition, true ); else xCursor->gotoEnd( true ); - //the paragraph after this new section is already inserted - xCursor->goLeft(1, true); - css::uno::Reference xTextRange(xCursor, css::uno::UNO_QUERY_THROW); + // The paragraph after this new section is already inserted. The previous node may be a + // table; then trying to go left would skip the whole table. Split the trailing + // paragraph; let the section span over the first of the two resulting paragraphs; + // destroy the last section's paragraph afterwards. + css::uno::Reference xEndPara = xCursor->getEnd(); + xTextAppend->insertControlCharacter( + xEndPara, css::text::ControlCharacter::PARAGRAPH_BREAK, false); + css::uno::Reference xNewPara = xCursor->getEnd(); + xCursor->gotoPreviousParagraph(true); + xEndPara = xCursor->getEnd(); + // xEndPara may already have properties (like page break); make sure to apply them + // to the newly appended paragraph, which will be kept in the end. + copyAllProps(xEndPara, xNewPara); - if (css::uno::Reference xIndexSupplier{ - GetTextDocument(), css::uno::UNO_QUERY }) + uno::Reference< text::XTextContent > xSection( m_xTextFactory->createInstance("com.sun.star.text.TextSection"), uno::UNO_QUERY_THROW ); + xSection->attach(xCursor); + + // Remove the extra paragraph (last inside the section) + if (uno::Reference xEA{ xEndPara, uno::UNO_QUERY }) { - css::uno::Reference xCompare( - xTextAppend, css::uno::UNO_QUERY); - const auto xIndexAccess = xIndexSupplier->getDocumentIndexes(); - for (sal_Int32 i = xIndexAccess->getCount(); i > 0; --i) + if (uno::Reference xParagraph{ + xEA->createEnumeration()->nextElement(), uno::UNO_QUERY }) { - if (css::uno::Reference xIndex{ - xIndexAccess->getByIndex(i - 1), css::uno::UNO_QUERY }) - { - const auto xIndexTextRange = xIndex->getAnchor(); - if (xCompare->compareRegionStarts(xTextRange, xIndexTextRange) == 0 - && xCompare->compareRegionEnds(xTextRange, xIndexTextRange) == 0) - { - // The boundaries coincide with an index: trying to attach a section - // to the range will insert the section inside the index. goRight will - // extend the range outside of the index, so that created section will - // be around it. Alternatively we could return index section itself - // instead : xRet.set(xIndex, uno::UNO_QUERY) - to set its properties, - // like columns/fill. - xCursor->goRight(1, true); - break; - } - } + xParagraph->dispose(); } } - uno::Reference< text::XTextContent > xSection( m_xTextFactory->createInstance("com.sun.star.text.TextSection"), uno::UNO_QUERY_THROW ); - xSection->attach( xTextRange ); xRet.set(xSection, uno::UNO_QUERY ); } catch(const uno::Exception&) -- cgit v1.2.3