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 <jluth@mail.com>
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 0000000..5440a81
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf127622_framePr.docx
Binary files differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index 6f84d97..caf312b 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 8511417..11247cb 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));
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(rAppendContext.pLastParagraphProperties->IsFrameMode()) } }));
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
RegisterFrameConversion(
rAppendContext.pLastParagraphProperties->GetStartingRange(),
rAppendContext.pLastParagraphProperties->GetEndingRange(),
std::move(aFrameProperties) );
}
catch( const uno::Exception& )
catch (const uno::Exception&)
{
}
return aFrameProperties;
}
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;
std::vector<beans::PropertyValue> aFrameProperties
= MakeFrameProperties(*rAppendContext.pLastParagraphProperties);
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 4731a8e..5d935b5 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 40d9eec..7794249 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 23c25be..b458b70 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; }
