sw/qa/extras/ooxmlexport/data/tdf76683_negativeTwipsMeasure.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport11.cxx | 11 +++
writerfilter/source/dmapper/PropertyMap.cxx | 13 ++-
writerfilter/source/ooxml/OOXMLFactory.cxx | 8 ++
writerfilter/source/ooxml/OOXMLFactory.hxx | 3
writerfilter/source/ooxml/factoryimpl.py | 3
writerfilter/source/ooxml/model.xml | 33
++++++++--
7 files changed, 59 insertions(+), 12 deletions(-)
New commits:
commit 93242e985a002d94b0ac765952ce47b928effcae
Author: Justin Luth <justin.l...@collabora.com>
AuthorDate: Thu Sep 13 15:01:30 2018 +0300
Commit: Justin Luth <justin_l...@sil.org>
CommitDate: Wed Oct 3 07:54:00 2018 +0200
tdf#76683 writerfilter: TwipMeasure must be positive
...and the column width must not be negative.
The previous column logic ensured that the total width was
larger than the reference by adding a fake buffer
and then subtracted the difference from the final column.
In the case of a zero-width final column, it could become negative.
The current logic ensures that the total width is less than
the reference width, and then adds the difference
(which should be a smaller difference now) to the final column.
Regression potential - early columns that need every single twip
of bonus space might not fit anymore. On the other hand,
ending columns might be fixed...
Change-Id: Ie75d455e8ed62dbec5a1b9c901417df8d842ace8
Reviewed-on: https://gerrit.libreoffice.org/59400
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_l...@sil.org>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf76683_negativeTwipsMeasure.docx
b/sw/qa/extras/ooxmlexport/data/tdf76683_negativeTwipsMeasure.docx
new file mode 100644
index 000000000000..eb769fdcc3bf
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf76683_negativeTwipsMeasure.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index db52d019439b..adb0dc6f4264 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -160,6 +160,17 @@ DECLARE_OOXMLEXPORT_TEST(testTdf82065_Ind_start_strict,
"tdf82065_Ind_start_stri
CPPUNIT_ASSERT_EQUAL_MESSAGE("IndentAt defined", true, bFoundIndentAt);
}
+DECLARE_OOXMLEXPORT_TEST(testTdf76683_negativeTwipsMeasure,
"tdf76683_negativeTwipsMeasure.docx")
+{
+ xmlDocPtr pXmlDoc = parseExport("word/document.xml");
+ if (!pXmlDoc)
+ return;
+ assertXPath(pXmlDoc, "/w:document/w:body/w:sectPr/w:cols/w:col", 2);
+ sal_uInt32 nColumn1 = getXPath(pXmlDoc,
"/w:document/w:body/w:sectPr/w:cols/w:col[1]", "w").toUInt32();
+ sal_uInt32 nColumn2 = getXPath(pXmlDoc,
"/w:document/w:body/w:sectPr/w:cols/w:col[2]", "w").toUInt32();
+ CPPUNIT_ASSERT( nColumn1 > nColumn2 );
+}
+
DECLARE_OOXMLEXPORT_TEST(testTdf112694, "tdf112694.docx")
{
uno::Any aPageStyle = getStyles("PageStyles")->getByName("Standard");
diff --git a/writerfilter/source/dmapper/PropertyMap.cxx
b/writerfilter/source/dmapper/PropertyMap.cxx
index 691f1a00a474..4048080e03c5 100644
--- a/writerfilter/source/dmapper/PropertyMap.cxx
+++ b/writerfilter/source/dmapper/PropertyMap.cxx
@@ -716,13 +716,18 @@ uno::Reference< text::XTextColumns >
SectionPropertyMap::ApplyColumnProperties(
nColSum = 0;
for ( sal_Int32 nCol = 0; nCol <= m_nColumnCount; ++nCol )
{
- pColumn[nCol].LeftMargin = nCol ? m_aColDistance[nCol - 1] / 2
: 0;
- pColumn[nCol].RightMargin = nCol == m_nColumnCount ? 0 :
m_aColDistance[nCol] / 2;
- pColumn[nCol].Width = sal_Int32( (double( m_aColWidth[nCol] +
pColumn[nCol].RightMargin + pColumn[nCol].LeftMargin ) + 0.5) * fRel );
+ const double fLeft = nCol ? m_aColDistance[nCol - 1] / 2 : 0;
+ pColumn[nCol].LeftMargin = fLeft;
+ const double fRight = nCol == m_nColumnCount ? 0 :
m_aColDistance[nCol] / 2;
+ pColumn[nCol].RightMargin = fRight;
+ const double fWidth = m_aColWidth[nCol];
+ pColumn[nCol].Width = (fWidth + fLeft + fRight) * fRel;
nColSum += pColumn[nCol].Width;
}
if ( nColSum != nRefValue )
- pColumn[m_nColumnCount].Width -= (nColSum - nRefValue);
+ pColumn[m_nColumnCount].Width += (nRefValue - nColSum);
+ assert( pColumn[m_nColumnCount].Width >= 0 );
+
xColumns->setColumns( aColumns );
}
else
diff --git a/writerfilter/source/ooxml/OOXMLFactory.cxx
b/writerfilter/source/ooxml/OOXMLFactory.cxx
index 4c41684cd594..d98d22db485b 100644
--- a/writerfilter/source/ooxml/OOXMLFactory.cxx
+++ b/writerfilter/source/ooxml/OOXMLFactory.cxx
@@ -102,10 +102,16 @@ void OOXMLFactory::attributes(OOXMLFastContextHandler *
pHandler,
pFactory->attributeAction(pHandler, nToken, xValue);
}
break;
- case ResourceType::TwipsMeasure:
+ case ResourceType::TwipsMeasure_asSigned:
+ case ResourceType::TwipsMeasure_asZero:
{
const char *pValue = pAttribs->getAsCharByIndex(nAttrIndex);
OOXMLValue::Pointer_t xValue(new
OOXMLTwipsMeasureValue(pValue));
+ if ( xValue->getInt() < 0 )
+ {
+ if ( pAttr->m_nResource ==
ResourceType::TwipsMeasure_asZero )
+ xValue = OOXMLIntegerValue::Create(0);
+ }
pHandler->newProperty(nId, xValue);
pFactory->attributeAction(pHandler, nToken, xValue);
}
diff --git a/writerfilter/source/ooxml/OOXMLFactory.hxx
b/writerfilter/source/ooxml/OOXMLFactory.hxx
index f1432869f4e2..04a71bbc92f4 100644
--- a/writerfilter/source/ooxml/OOXMLFactory.hxx
+++ b/writerfilter/source/ooxml/OOXMLFactory.hxx
@@ -52,7 +52,8 @@ enum class ResourceType {
PropertyTable,
Math,
Any,
- TwipsMeasure,
+ TwipsMeasure_asSigned,
+ TwipsMeasure_asZero,
HpsMeasure,
MeasurementOrPercent
};
diff --git a/writerfilter/source/ooxml/factoryimpl.py
b/writerfilter/source/ooxml/factoryimpl.py
index 2d54ee8ff6b8..5f397ab9cbad 100644
--- a/writerfilter/source/ooxml/factoryimpl.py
+++ b/writerfilter/source/ooxml/factoryimpl.py
@@ -38,7 +38,8 @@ def createFastChildContextFromFactory(model):
switch (nResource)
{""")
resources = [
- "List", "Integer", "Hex", "HexColor", "String", "TwipsMeasure",
+ "List", "Integer", "Hex", "HexColor", "String",
+ "TwipsMeasure_asSigned", "TwipsMeasure_asZero",
"HpsMeasure", "Boolean", "MeasurementOrPercent",
]
for resource in [r.getAttribute("resource") for r in
model.getElementsByTagName("resource")]:
diff --git a/writerfilter/source/ooxml/model.xml
b/writerfilter/source/ooxml/model.xml
index e1640c44e54d..953010eb020b 100644
--- a/writerfilter/source/ooxml/model.xml
+++ b/writerfilter/source/ooxml/model.xml
@@ -8236,7 +8236,7 @@
<resource name="CT_OMathJc" resource="Value">
<attribute name="val" tokenid="ooxml:CT_OMathJc_val" action="setValue"/>
</resource>
- <resource name="ST_TwipsMeasure" resource="TwipsMeasure"/>
+ <resource name="ST_TwipsMeasure" resource="TwipsMeasure_asSigned"/>
<resource name="CT_TwipsMeasure" resource="Value">
<attribute name="val" tokenid="ooxml:CT_TwipsMeasure_val"
action="setValue"/>
<action name="start" action="setDefaultIntegerValue"/>
@@ -10683,9 +10683,25 @@
<define name="ST_UnsignedDecimalNumber">
<data type="unsignedLong"/>
</define>
+<!-- MS documentation states that TwipsMeasure is a positive value
+ but it doesn't indicate how to handle an invalid negative
+ value. So, ST_TwipsMeasure matches the documented type,
+ and the extension (_asSigned, _asZero, or perhaps _asAbsolute)
+ indicates how MS handles it in practice.
+
+ Historically, LO has treated TwipsMeasure as signed,
+ i.e. that negative numbers are allowed and treated as negative,
+ so that is the default handling.
+-->
<define name="ST_TwipsMeasure">
<data type="unsignedLong"/>
</define>
+ <define name="ST_TwipsMeasure_asSigned">
+ <data type="unsignedLong"/>
+ </define>
+ <define name="ST_TwipsMeasure_asZero">
+ <data type="unsignedLong"/>
+ </define>
<define name="CT_TwipsMeasure">
<attribute name="val">
<ref name="ST_TwipsMeasure"/>
@@ -13013,10 +13029,10 @@
</define>
<define name="CT_Column">
<attribute name="w">
- <ref name="ST_TwipsMeasure"/>
+ <ref name="ST_TwipsMeasure_asZero"/>
</attribute>
<attribute name="space">
- <ref name="ST_TwipsMeasure"/>
+ <ref name="ST_TwipsMeasure_asZero"/>
</attribute>
</define>
<define name="CT_Columns">
@@ -16656,12 +16672,19 @@
<action name="start" action="setDefaultIntegerValue"/>
</resource>
<resource name="ST_UnsignedDecimalNumber" resource="Integer"/>
- <resource name="ST_TwipsMeasure" resource="TwipsMeasure"/>
+<!--
+ Historically, LO has treated TwipsMeasure as signed,
+ i.e. that negative numbers are allowed and treated as negative,
+ so that is the default handling.
+-->
+ <resource name="ST_TwipsMeasure" resource="TwipsMeasure_asSigned"/>
+ <resource name="ST_TwipsMeasure_asSigned"
resource="TwipsMeasure_asSigned"/>
+ <resource name="ST_TwipsMeasure_asZero" resource="TwipsMeasure_asZero"/>
<resource name="CT_TwipsMeasure" resource="Value">
<attribute name="val" tokenid="ooxml:CT_TwipsMeasure_val"
action="setValue"/>
<action name="start" action="setDefaultIntegerValue"/>
</resource>
- <resource name="ST_SignedTwipsMeasure" resource="TwipsMeasure"/>
+ <resource name="ST_SignedTwipsMeasure" resource="TwipsMeasure_asSigned"/>
<resource name="CT_SignedTwipsMeasure" resource="Value">
<attribute name="val" tokenid="ooxml:CT_SignedTwipsMeasure_val"
action="setValue"/>
<action name="start" action="setDefaultIntegerValue"/>
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
