[Libreoffice-commits] core.git: 2 commits - libreofficekit/qa sw/inc sw/qa sw/source
László Németh (via logerrit)
logerrit at kemper.freedesktop.org
Fri Sep 4 12:25:07 UTC 2020
libreofficekit/qa/unit/tiledrendering.cxx | 16 +++---
sw/inc/calc.hxx | 5 +-
sw/qa/extras/ooxmlexport/data/tdf136404.fodt | 65 +++++++++++++++++++++++++++
sw/qa/extras/ooxmlexport/ooxmlexport15.cxx | 42 +++++++++++++++++
sw/source/core/bastyp/calc.cxx | 17 +++++--
sw/source/core/fields/cellfml.cxx | 35 ++++++++++++--
6 files changed, 164 insertions(+), 16 deletions(-)
New commits:
commit 7dbd1cd44918c50f2540955f908cd0a96fce024c
Author: László Németh <nemeth at numbertext.org>
AuthorDate: Thu Sep 3 12:17:46 2020 +0200
Commit: László Németh <nemeth at numbertext.org>
CommitDate: Fri Sep 4 14:24:39 2020 +0200
tdf#136404 DOCX import: ignore NaN cells in table formula
Ignore empty cells or cells with text content in data range
of AVERAGE, COUNT and PRODUCT (the new interoperability
functions in Writer), like MSO does, instead of using
NaN data as zeroes here.
Add AVERAGE, as a new function instead of alias of MEAN
to return error message instead of zero for NaN-only
arguments, like Calc does (Note: also MSO gives empty
result instead of zero).
Change-Id: I5e18f3e2b16cb0621dad2bba141ae63fb5edd241
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102012
Tested-by: Jenkins
Reviewed-by: László Németh <nemeth at numbertext.org>
diff --git a/sw/inc/calc.hxx b/sw/inc/calc.hxx
index 99c024bcac35..b7bb83af26a8 100644
--- a/sw/inc/calc.hxx
+++ b/sw/inc/calc.hxx
@@ -98,6 +98,7 @@ extern const char sCalc_Abs[];
enum class SwCalcError
{
NONE=0,
+ NaN, // not a number (not an error, used for interoperability)
Syntax, // syntax error
DivByZero, // division by zero
FaultyBrackets, // faulty brackets
@@ -204,6 +205,7 @@ class SwCalc
CharClass* m_pCharClass;
sal_uInt16 m_nListPor;
+ bool m_bHasNumber; // fix COUNT() and AVERAGE(), if all cells are NaN
SwCalcOper m_eCurrOper;
SwCalcOper m_eCurrListOper;
SwCalcError m_eError;
@@ -240,7 +242,8 @@ public:
CharClass* GetCharClass();
void SetCalcError( SwCalcError eErr ) { m_eError = eErr; }
- bool IsCalcError() const { return SwCalcError::NONE != m_eError; }
+ bool IsCalcError() const { return SwCalcError::NONE != m_eError && SwCalcError::NaN != m_eError; }
+ bool IsCalcNotANumber() const { return SwCalcError::NaN == m_eError; }
static bool Str2Double( const OUString& rStr, sal_Int32& rPos,
double& rVal );
diff --git a/sw/qa/extras/ooxmlexport/data/tdf136404.fodt b/sw/qa/extras/ooxmlexport/data/tdf136404.fodt
new file mode 100644
index 000000000000..9ac902d45b3f
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf136404.fodt
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:table="urn:oasis:names:tc:opendocument:xmlns:table:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0" xmlns:officeooo="http://openoffice.org/2009/office" xmlns:loext="urn:org:documentfoundation:names:experimental:office:xmlns:loext:1.0" xmlns:ooow="http://openoffice.org/2004/writer" office:version="1.2" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:styles>
+ <style:style style:name="Standard" style:family="paragraph" style:class="text"/>
+ <style:default-style style:family="paragraph">
+ <style:text-properties fo:language="en" fo:country="US"/>
+ </style:default-style>
+ </office:styles>
+ <office:body>
+ <office:text>
+ <table:table>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-column/>
+ <table:table-row>
+ <table:table-cell office:value-type="string">
+ <text:p>6</text:p>
+ </table:table-cell>
+ <table:table-cell office:value-type="string">
+ <text:p></text:p>
+ </table:table-cell>
+ <table:table-cell office:value-type="string">
+ <text:p></text:p>
+ </table:table-cell>
+ <table:table-cell office:value-type="string">
+ <text:p>10</text:p>
+ </table:table-cell>
+ <table:table-cell office:value-type="string">
+ <text:p>text</text:p>
+ </table:table-cell>
+ <table:table-cell office:value-type="string">
+ <text:p>text</text:p>
+ </table:table-cell>
+ </table:table-row>
+ <table:table-row>
+ <table:table-cell table:formula="ooow:COUNT(<A1:F1>)" office:value-type="float" office:value="2">
+ <text:p>2</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:AVERAGE(<A1:F1>)" office:value-type="float" office:value="0">
+ <text:p>8</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:PRODUCT(<A1:F1>)" office:value-type="float" office:value="60">
+ <text:p>60</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:COUNT(<B1>)" office:value-type="float" office:value="0">
+ <text:p>0</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:COUNT(<B1:C1>)" office:value-type="float" office:value="0">
+ <text:p>0</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:AVERAGE(<B1>)" office:value-type="float" office:value="1.79769313486232E+308">
+ <text:p text:style-name="P1">** Expression is faulty **</text:p>
+ </table:table-cell>
+ <table:table-cell table:formula="ooow:AVERAGE(<B1:C1>)" office:value-type="float" office:value="1.79769313486232E+308">
+ <text:p text:style-name="P1">** Expression is faulty **</text:p>
+ </table:table-cell>
+ </table:table-row>
+ </table:table>
+ </office:text>
+ </office:body>
+</office:document>
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
index 48cf77e84f0b..8d6e8228f8ba 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
@@ -173,6 +173,48 @@ DECLARE_OOXMLEXPORT_TEST(testTdf123356, "tdf123356.fodt")
CPPUNIT_ASSERT_EQUAL(OUString("4"), xEnumerationAccess2->getPresentation(false).trim());
}
+DECLARE_OOXMLEXPORT_TEST(testTdf136404, "tdf136404.fodt")
+{
+ uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
+ uno::Reference<container::XEnumerationAccess> xFieldsAccess(xTextFieldsSupplier->getTextFields());
+ uno::Reference<container::XEnumeration> xFields(xFieldsAccess->createEnumeration());
+
+ // Ignore empty cells or cells with text content with new interoperability functions COUNT, AVERAGE and PRODUCT
+ uno::Reference<text::XTextField> xEnumerationAccess1(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<A1:F1>)"), xEnumerationAccess1->getPresentation(true).trim());
+ CPPUNIT_ASSERT_EQUAL(OUString("2"), xEnumerationAccess1->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess2(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<B1:C1>)"), xEnumerationAccess2->getPresentation(true).trim());
+ // This was 0
+ CPPUNIT_ASSERT_EQUAL(OUString("** Expression is faulty **"), xEnumerationAccess2->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess3(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<B1>)"), xEnumerationAccess3->getPresentation(true).trim());
+ // This was 0
+ CPPUNIT_ASSERT_EQUAL(OUString("** Expression is faulty **"), xEnumerationAccess3->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess4(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<B1:C1>)"), xEnumerationAccess4->getPresentation(true).trim());
+ // This was 2
+ CPPUNIT_ASSERT_EQUAL(OUString("0"), xEnumerationAccess4->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess5(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("COUNT(<B1>)"), xEnumerationAccess5->getPresentation(true).trim());
+ // This was 1
+ CPPUNIT_ASSERT_EQUAL(OUString("0"), xEnumerationAccess5->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess6(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("PRODUCT(<A1:F1>)"), xEnumerationAccess6->getPresentation(true).trim());
+ // This was 0
+ CPPUNIT_ASSERT_EQUAL(OUString("60"), xEnumerationAccess6->getPresentation(false).trim());
+
+ uno::Reference<text::XTextField> xEnumerationAccess7(xFields->nextElement(), uno::UNO_QUERY);
+ CPPUNIT_ASSERT_EQUAL(OUString("AVERAGE(<A1:F1>)"), xEnumerationAccess7->getPresentation(true).trim());
+ // This was 2
+ CPPUNIT_ASSERT_EQUAL(OUString("8"), xEnumerationAccess7->getPresentation(false).trim());
+}
+
DECLARE_OOXMLEXPORT_TEST(testTdf123390, "tdf123390.fodt")
{
uno::Reference<text::XTextFieldsSupplier> xTextFieldsSupplier(mxComponent, uno::UNO_QUERY);
diff --git a/sw/source/core/bastyp/calc.cxx b/sw/source/core/bastyp/calc.cxx
index 17179f472352..41bf202a9ea4 100644
--- a/sw/source/core/bastyp/calc.cxx
+++ b/sw/source/core/bastyp/calc.cxx
@@ -106,7 +106,7 @@ CalcOp const aOpTable[] = {
/* AND */ {{sCalc_And}, CALC_AND}, // log. AND
/* ASIN */ {{sCalc_Asin}, CALC_ASIN}, // Arc sine
/* ATAN */ {{sCalc_Atan}, CALC_ATAN}, // Arc tangent
-/* AVERAGE */ {{sCalc_Average}, CALC_MEAN}, // Mean (since LibreOffice 7.1)
+/* AVERAGE */ {{sCalc_Average}, CALC_AVERAGE}, // Average (since LibreOffice 7.1)
/* COS */ {{sCalc_Cos}, CALC_COS}, // Cosine
/* COUNT */ {{sCalc_Count}, CALC_COUNT}, // Count (since LibreOffice 7.1)
/* DATE */ {{sCalc_Date}, CALC_DATE}, // Date
@@ -221,6 +221,7 @@ SwCalc::SwCalc( SwDoc& rD )
, m_pLocaleDataWrapper( m_aSysLocale.GetLocaleDataPtr() )
, m_pCharClass( &GetAppCharClass() )
, m_nListPor( 0 )
+ , m_bHasNumber( false )
, m_eCurrOper( CALC_NAME )
, m_eCurrListOper( CALC_NAME )
, m_eError( SwCalcError::NONE )
@@ -460,6 +461,7 @@ SwCalcExp* SwCalc::VarLook( const OUString& rStr, bool bIns )
{
// Save the current values...
sal_uInt16 nListPor = m_nListPor;
+ bool bHasNumber = m_bHasNumber;
SwSbxValue nLastLeft = m_nLastLeft;
SwSbxValue nNumberValue = m_nNumberValue;
sal_Int32 nCommandPos = m_nCommandPos;
@@ -471,6 +473,7 @@ SwCalcExp* SwCalc::VarLook( const OUString& rStr, bool bIns )
// ...and write them back.
m_nListPor = nListPor;
+ m_bHasNumber = bHasNumber;
m_nLastLeft = nLastLeft;
m_nNumberValue = nNumberValue;
m_nCommandPos = nCommandPos;
@@ -674,6 +677,7 @@ SwCalcOper SwCalc::GetToken()
{
case CALC_SUM:
case CALC_MEAN:
+ case CALC_AVERAGE:
case CALC_COUNT:
m_eCurrListOper = CALC_PLUS;
break;
@@ -1120,6 +1124,7 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
{
SAL_INFO("sw.calc", "number: " << m_nNumberValue.GetDouble());
SwSbxValue nErg;
+ m_bHasNumber = true;
if( GetToken() == CALC_PHD )
{
double aTmp = m_nNumberValue.GetDouble();
@@ -1196,14 +1201,19 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
SAL_INFO("sw.calc", ")");
break;
case CALC_MEAN:
+ case CALC_AVERAGE:
{
SAL_INFO("sw.calc", "mean");
m_nListPor = 1;
+ m_bHasNumber = CALC_MEAN == m_eCurrOper;
GetToken();
SwSbxValue nErg = Expr();
double aTmp = nErg.GetDouble();
aTmp /= m_nListPor;
- nErg.PutDouble( aTmp );
+ if ( !m_bHasNumber )
+ m_eError = SwCalcError::DivByZero;
+ else
+ nErg.PutDouble( aTmp );
return nErg;
break;
}
@@ -1211,9 +1221,10 @@ SwSbxValue SwCalc::PrimFunc(bool &rChkPow)
{
SAL_INFO("sw.calc", "count");
m_nListPor = 1;
+ m_bHasNumber = false;
GetToken();
SwSbxValue nErg = Expr();
- nErg.PutDouble( m_nListPor );
+ nErg.PutDouble( m_bHasNumber ? m_nListPor : 0 );
return nErg;
break;
}
diff --git a/sw/source/core/fields/cellfml.cxx b/sw/source/core/fields/cellfml.cxx
index 755d81f23fcc..47cbabe871a5 100644
--- a/sw/source/core/fields/cellfml.cxx
+++ b/sw/source/core/fields/cellfml.cxx
@@ -238,6 +238,8 @@ double SwTableBox::GetValue( SwTableCalcPara& rCalcPara ) const
if( pDoc->IsNumberFormat( sText, nFormatIndex, aNum ))
nRet = aNum;
+ else
+ rCalcPara.m_rCalc.SetCalcError( SwCalcError::NaN ); // set for interoperability functions
}
// ?? otherwise it is an error
} while( false );
@@ -354,6 +356,9 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
SwSelBoxes aBoxes;
GetBoxes( *pSttBox, *pEndBox, aBoxes );
+ // don't use empty cells or cells with text content as zeroes in interoperability functions
+ sal_Int16 nUseOnlyNumber = -1;
+
rNewStr.append("(");
bool bDelim = false;
for (size_t n = 0; n < aBoxes.size() &&
@@ -362,11 +367,26 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
const SwTableBox* pTableBox = aBoxes[n];
if ( pTableBox->getRowSpan() >= 1 )
{
+ double fVal = pTableBox->GetValue( *pCalcPara );
+
+ if ( pCalcPara->m_rCalc.IsCalcNotANumber() )
+ {
+ if ( nUseOnlyNumber == -1 )
+ {
+ OUString sFormula = rNewStr.toString().toAsciiUpperCase();
+ nUseOnlyNumber = sal_Int16(
+ sFormula.lastIndexOf("AVERAGE") > -1 ||
+ sFormula.lastIndexOf("COUNT") > -1 ||
+ sFormula.lastIndexOf("PRODUCT") > -1 );
+ }
+ if ( nUseOnlyNumber > 0 )
+ continue;
+ }
+
if( bDelim )
rNewStr.append(cListDelim);
bDelim = true;
- rNewStr.append(pCalcPara->m_rCalc.GetStrResult(
- pTableBox->GetValue( *pCalcPara ) ));
+ rNewStr.append(pCalcPara->m_rCalc.GetStrResult( fVal ));
}
}
rNewStr.append(")");
@@ -378,8 +398,15 @@ void SwTableFormula::MakeFormula_( const SwTable& rTable, OUStringBuffer& rNewSt
if ( pSttBox->getRowSpan() >= 1 )
{
rNewStr.append("(");
- rNewStr.append(pCalcPara->m_rCalc.GetStrResult(
- pSttBox->GetValue( *pCalcPara ) ));
+ double fVal = pSttBox->GetValue( *pCalcPara );
+ // don't use empty cell or a cell with text content as zero in interoperability functions
+ // (except PRODUCT, where the result is correct anyway)
+ if ( !pCalcPara->m_rCalc.IsCalcNotANumber() ||
+ ( rNewStr.toString().toAsciiUpperCase().lastIndexOf("AVERAGE") == -1 &&
+ rNewStr.toString().toAsciiUpperCase().lastIndexOf("COUNT") == -1 ) )
+ {
+ rNewStr.append(pCalcPara->m_rCalc.GetStrResult( fVal ));
+ }
rNewStr.append(")");
}
}
commit bcb415d56de44246eea8c9de45e7f9f91b9d6570
Author: Tor Lillqvist <tml at collabora.com>
AuthorDate: Fri Sep 4 13:40:48 2020 +0300
Commit: Tor Lillqvist <tml at collabora.com>
CommitDate: Fri Sep 4 14:24:25 2020 +0200
Avoid loplugin:external and loplugin:simplifypointertobool warnings
Note that this file isn't actually compiled at the moment unless you
edit the Module_libreofficekit.mk. And then the unit test will fail.
But that is another problem.
Change-Id: I431ce889ca4059e98662cddcaee3aa570102c515
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102042
Tested-by: Jenkins
Reviewed-by: Tor Lillqvist <tml at collabora.com>
diff --git a/libreofficekit/qa/unit/tiledrendering.cxx b/libreofficekit/qa/unit/tiledrendering.cxx
index ee935cf82cdb..56d789b61e5e 100644
--- a/libreofficekit/qa/unit/tiledrendering.cxx
+++ b/libreofficekit/qa/unit/tiledrendering.cxx
@@ -67,7 +67,7 @@ void insertString(Document& rDocument, const std::string& s)
}
-OUString getFileURLFromSystemPath(OUString const & path)
+static OUString getFileURLFromSystemPath(OUString const & path)
{
OUString url;
osl::FileBase::RC e = osl::FileBase::getFileURLFromSystemPath(path, url);
@@ -130,7 +130,7 @@ void TiledRenderingTest::runAllTests()
std::unique_ptr< Office > pOffice( lok_cpp_init(
m_sLOPath.c_str() ) );
- CPPUNIT_ASSERT( pOffice.get() );
+ CPPUNIT_ASSERT( pOffice );
testDocumentLoadFail( pOffice.get() );
testDocumentTypes( pOffice.get() );
@@ -148,7 +148,7 @@ void TiledRenderingTest::testDocumentLoadFail( Office* pOffice )
{
const string sDocPath = m_sSrcRoot + "/libreofficekit/qa/data/IDONOTEXIST.odt";
std::unique_ptr< Document> pDocument( pOffice->documentLoad( sDocPath.c_str() ) );
- CPPUNIT_ASSERT( !pDocument.get() );
+ CPPUNIT_ASSERT( !pDocument );
// TODO: we probably want to have some way of returning what
// the cause of failure was. getError() will return
// something along the lines of:
@@ -158,10 +158,10 @@ void TiledRenderingTest::testDocumentLoadFail( Office* pOffice )
// Our dumped .png files end up in
// workdir/CppunitTest/libreofficekit_tiledrendering.test.core
-int getDocumentType( Office* pOffice, const string& rPath )
+static int getDocumentType( Office* pOffice, const string& rPath )
{
std::unique_ptr< Document> pDocument( pOffice->documentLoad( rPath.c_str() ) );
- CPPUNIT_ASSERT( pDocument.get() );
+ CPPUNIT_ASSERT( pDocument );
return pDocument->getDocumentType();
}
@@ -180,7 +180,7 @@ void TiledRenderingTest::testDocumentTypes( Office* pOffice )
{
std::unique_ptr<Document> pDocument(loadDocument(pOffice, "blank_text.odt"));
- CPPUNIT_ASSERT(pDocument.get());
+ CPPUNIT_ASSERT(pDocument);
CPPUNIT_ASSERT_EQUAL(LOK_DOCTYPE_TEXT, static_cast<LibreOfficeKitDocumentType>(pDocument->getDocumentType()));
// This crashed.
pDocument->postUnoCommand(".uno:Bold");
@@ -223,7 +223,7 @@ void TiledRenderingTest::testPaintPartTile(Office* pOffice)
{
std::unique_ptr<Document> pDocument(loadDocument(pOffice, "blank_text.odt"));
- CPPUNIT_ASSERT(pDocument.get());
+ CPPUNIT_ASSERT(pDocument);
CPPUNIT_ASSERT_EQUAL(LOK_DOCTYPE_TEXT, static_cast<LibreOfficeKitDocumentType>(pDocument->getDocumentType()));
// Create two views.
@@ -401,7 +401,7 @@ void TiledRenderingTest::testMultiKeyInput(Office *pOffice)
{
std::unique_ptr<Document> pDocument(loadDocument(pOffice, "blank_text.odt"));
- CPPUNIT_ASSERT(pDocument.get());
+ CPPUNIT_ASSERT(pDocument);
CPPUNIT_ASSERT_EQUAL(LOK_DOCTYPE_TEXT, static_cast<LibreOfficeKitDocumentType>(pDocument->getDocumentType()));
// Create two views.
More information about the Libreoffice-commits
mailing list