tdf#124741: export default column width to XLSX

For some reason, we have never exported the default column width
to XLSX, although we have such export for XLS. This led to bugs
like tdf#100946. Workarounds applied to those bugs have made LO
to export columns which shouldn't have been exported, abusing
"customWidth" attribute in XclExpColinfo::XclExpColinfo depending
on if the column has size different from an app-global default;
after that, sheet-local default was determined, and then columns
which have mbCustomWidth set (i.e., different from app default),
as well as those different from sheet-local default, were stored.
Effectively, the hack had disabled the removal of defaults from
maColInfos in XclExpColinfoBuffer::Finalize. We even had unit
tests explicitly testing that we export those columns that Excel
skips. The effect of that is not only unnecessary data in the file;
the data was actually wrong (customWidth actually means that the
width was edited manually by user, even if equal to the default,
thus changing Excel handling of the column); and also Calc
initializes all columns to the right of last used column for such
a file. Only in case when app-global default happened to match
sheet-local one, columns would have properties allowing them to
be removed from maColInfos in the end of XclExpColinfoBuffer::Finalize,
which still resulted in problems similar to the workarounded one.

This patch implements proper export of the default column width
to XLSX, thus partially reverting changes made for tdf#100946 in
commit 40d892a2db4d750aaf0562c63004e693c028273c. Actually, our
export to XLSX does not depend on the 5-pixel correction (see
ECMA-376-1:2016 18.3.1.81), since the exported default depends
only on most-used column width. XclExpDefcolwidth implementation
was edited to only take the correction into account when exporting
to XLS (to keep status quo; it's clear that XLS widths export
and import implementation is incorrect: some empirical formula
is used in XclTools::GetXclDefColWidthCorrection, that was
introduced in commit 555d702903fb0857122024e1ab78a72d122d3f16 for
i#3006, that doesn't match any documentation, and changes widths
of columns in exported XLS - something to be fixed independently).

Change-Id: I227aca17e56247cbb839444717066a898987c4f1
Reviewed-on: https://gerrit.libreoffice.org/71132
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/71222

3 files changed, 69 insertions, 87 deletions

diff --git a/sc/qa/unit/subsequent_export-test.cxx b/sc/qa/unit/subsequent_export-test.cxx
index 787f3e43f13d..fd3b6c3eaa04 100644
--- a/sc/qa/unit/subsequent_export-test.cxx
+++ b/sc/qa/unit/subsequent_export-test.cxx
@@ -723,80 +723,60 @@ void ScExportTest::testCustomColumnWidthExportXLSX()
     xmlDocPtr pSheet = XPathHelper::parseExport(pXPathFile, m_xSFactory, "xl/worksheets/sheet1.xml");
     CPPUNIT_ASSERT(pSheet);
 
-    // First column, has everything default (width in Calc: 1280)
+    // tdf#124741: check that we export default width, otherwise the skipped columns would have
+    // wrong width. Previously defaultColWidth attribute was missing
+    double nDefWidth
+        = getXPath(pSheet, "/x:worksheet/x:sheetFormatPr", "defaultColWidth").toDouble();
+    CPPUNIT_ASSERT_DOUBLES_EQUAL(11.53515625, nDefWidth, 0.01);
+
+    // First column, has everything default (width in Calc: 1280), skipped
+
+    // Second column, has custom width (width in Calc: 1225)
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "hidden", "false");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "customWidth", "false");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "customWidth", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "min", "1");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "max", "1");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "min", "2");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[1]", "max", "2");
 
-    // Second column, has custom width (width in Calc: 1225)
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "hidden", "false");
+    // Third column, has everything default (width in Calc: 1280), skipped
+
+    // Fourth column has custom width. Columns from 4 to 7 are hidden
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "hidden", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "outlineLevel", "0");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "customWidth", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "min", "2");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "max", "2");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "min", "4");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[2]", "max", "4");
 
-    // Third column, has everything default (width in Calc: 1280)
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "hidden", "false");
+    // 5th column has custom width. Columns from 4 to 7 are hidden
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "hidden", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "customWidth", "false");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "customWidth", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "min", "3");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "max", "3");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "min", "5");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[3]", "max", "5");
 
-    // Fourth column has custom width. Columns from 4 to 7 are hidden
+    // 6th and 7th columns have default width and they are hidden
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "hidden", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "customWidth", "true");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "customWidth", "false");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "min", "4");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "max", "4");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "min", "6");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[4]", "max", "7");
 
-    // 5th column has custom width. Columns from 4 to 7 are hidden
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "hidden", "true");
+    // 8th column has everything default - skipped
+
+    // 9th column has custom width
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "hidden", "false");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "outlineLevel", "0");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "customWidth", "true");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "min", "5");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "max", "5");
-
-    // 6th and 7th columns has default width and it are hidden
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "hidden", "true");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "customWidth", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "min", "6");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[6]", "max", "7");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "min", "9");
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "max", "9");
 
-    // 8th column has everything default
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "hidden", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "customWidth", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "min", "8");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[7]", "max", "8");
-
-    // 9th column has custom width
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "hidden", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "customWidth", "true");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "min", "9");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[8]", "max", "9");
-
-    // Rest of columns are default
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "hidden", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "customWidth", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "min", "10");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[9]", "max", "1025");
-
-    // We expected that exactly 9 unique Nodes will be produced
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 9);
+    // We expected that exactly 5 unique Nodes will be produced
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 5);
 
     assertXPath(pSheet, "/x:worksheet/x:sheetData/x:row[1]", "hidden", "false");
     assertXPath(pSheet, "/x:worksheet/x:sheetData/x:row[1]", "outlineLevel", "0");
@@ -861,10 +841,7 @@ void ScExportTest::testColumnWidthResaveXLSX()
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "width", "250");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[5]", "customWidth", "true");
 
-    // The last column [6] is not existing in Excel sheet, and it is added only by LibreOffice.
-    // This column width is default and it is depended on operating system.
-
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 6);
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 5);
 }
 
 #if HAVE_MORE_FONTS
@@ -1024,13 +1001,8 @@ void ScExportTest::testOutlineExportXLSX()
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[12]", "min", "25");
     assertXPath(pSheet, "/x:worksheet/x:cols/x:col[12]", "max", "26");
 
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[13]", "hidden", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[13]", "outlineLevel", "0");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[13]", "collapsed", "false");
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col[13]", "min", "27");
-
-    // We expected that exactly 13 unique Nodes will be produced
-    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 13);
+    // We expected that exactly 12 unique Nodes will be produced
+    assertXPath(pSheet, "/x:worksheet/x:cols/x:col", 12);
 
     // We need to save all 30 rows, as it provides information about outLineLevel
     assertXPath(pSheet, "/x:worksheet/x:sheetData/x:row[1]", "r", "1");
diff --git a/sc/source/filter/excel/xetable.cxx b/sc/source/filter/excel/xetable.cxx
index 1fc75abd4380..a63c03567f70 100644
--- a/sc/source/filter/excel/xetable.cxx
+++ b/sc/source/filter/excel/xetable.cxx
@@ -1552,38 +1552,48 @@ void XclExpDimensions::WriteBody( XclExpStream& rStrm )
 
 namespace {
 
-double lclGetCorrectedColWidth( const XclExpRoot& rRoot, sal_uInt16 nXclColWidth )
+double lclGetCChCorrection(const XclExpRoot& rRoot)
 {
+    // Convert the correction from 1/256ths of a character size to count of chars
+    // TODO: make to fit ECMA-376-1:2016 18.3.1.81 sheetFormatPr (Sheet Format Properties):
+    // 5 pixels are added to the base width: 2 for margin padding on each side, plus 1 for gridline
+    // So this should depend on rRoot.GetCharWidth(), not on font height
+
     long nFontHt = rRoot.GetFontBuffer().GetAppFontData().mnHeight;
-    return nXclColWidth - XclTools::GetXclDefColWidthCorrection( nFontHt );
+    return XclTools::GetXclDefColWidthCorrection(nFontHt) / 256.0;
 }
 
 } // namespace
 
 XclExpDefcolwidth::XclExpDefcolwidth( const XclExpRoot& rRoot ) :
-    XclExpUInt16Record( EXC_ID_DEFCOLWIDTH, EXC_DEFCOLWIDTH_DEF ),
+    XclExpDoubleRecord(EXC_ID_DEFCOLWIDTH, EXC_DEFCOLWIDTH_DEF + lclGetCChCorrection(rRoot)),
     XclExpRoot( rRoot )
 {
 }
 
 bool XclExpDefcolwidth::IsDefWidth( sal_uInt16 nXclColWidth ) const
 {
-    double fNewColWidth = lclGetCorrectedColWidth( GetRoot(), nXclColWidth );
     // This formula is taking number of characters with GetValue()
-    // and it is translating it into default column width. 0.5 means half character.
+    // and it is translating it into default column width.
     // https://msdn.microsoft.com/en-us/library/documentformat.openxml.spreadsheet.column.aspx
-    long defaultColumnWidth = static_cast< long >( 256.0 * ( GetValue() + 0.5 ) );
+    double defaultColumnWidth = 256.0 * GetValue();
 
     // exactly matched, if difference is less than 1/16 of a character to the left or to the right
-    return std::abs( defaultColumnWidth - fNewColWidth ) < 16;
+    return std::abs(defaultColumnWidth - nXclColWidth) < 256.0 * 1.0 / 16.0;
 }
 
 void XclExpDefcolwidth::SetDefWidth( sal_uInt16 nXclColWidth )
 {
-    double fNewColWidth = lclGetCorrectedColWidth( GetRoot(), nXclColWidth );
-    // This function is taking width and translate it into number of characters
-    // Next this number of characters are stored. 0.5 means half character.
-    SetValue( limit_cast< sal_uInt16 >( fNewColWidth / 256.0 - 0.5 ) );
+    SetValue(nXclColWidth / 256.0);
+}
+
+void XclExpDefcolwidth::Save(XclExpStream& rStrm)
+{
+    double fCorrectedCCh = GetValue() - lclGetCChCorrection(GetRoot());
+    // Convert double to sal_uInt16
+    XclExpUInt16Record aUInt16Rec(GetRecId(),
+                                  static_cast<sal_uInt16>(std::round(fCorrectedCCh)));
+    aUInt16Rec.Save(rStrm);
 }
 
 XclExpColinfo::XclExpColinfo( const XclExpRoot& rRoot,
@@ -1613,10 +1623,6 @@ XclExpColinfo::XclExpColinfo( const XclExpRoot& rRoot,
     // column flags
     ::set_flag( mnFlags, EXC_COLINFO_HIDDEN, rDoc.ColHidden(nScCol, nScTab) );
 
-    XclExpDefcolwidth defColWidth = XclExpDefcolwidth( rRoot );
-    mbCustomWidth = !defColWidth.IsDefWidth( mnWidth );
-    set_flag(mnFlags, EXC_COLINFO_CUSTOMWIDTH, mbCustomWidth);
-
     // outline data
     rOutlineBfr.Update( nScCol );
     ::set_flag( mnFlags, EXC_COLINFO_COLLAPSED, rOutlineBfr.IsCollapsed() );
@@ -1629,12 +1635,13 @@ void XclExpColinfo::ConvertXFIndexes()
     maXFId.ConvertXFIndex( GetRoot() );
 }
 
-bool XclExpColinfo::IsDefault( const XclExpDefcolwidth& rDefColWidth ) const
+bool XclExpColinfo::IsDefault( const XclExpDefcolwidth& rDefColWidth )
 {
+    mbCustomWidth = !rDefColWidth.IsDefWidth(mnWidth);
     return (maXFId.mnXFIndex == EXC_XF_DEFAULTCELL) &&
            (mnFlags == 0) &&
            (mnOutlineLevel == 0) &&
-           rDefColWidth.IsDefWidth( mnWidth );
+           !mbCustomWidth;
 }
 
 bool XclExpColinfo::TryMerge( const XclExpColinfo& rColInfo )
@@ -2701,7 +2708,7 @@ void XclExpCellTable::SaveXml( XclExpXmlStream& rStrm )
     sax_fastparser::FSHelperPtr& rWorksheet = rStrm.GetCurrentStream();
     rWorksheet->startElement( XML_sheetFormatPr,
         // OOXTODO: XML_baseColWidth
-        // OOXTODO: XML_defaultColWidth
+        XML_defaultColWidth, OString::number(maColInfoBfr.GetDefColWidth()),
         // OOXTODO: XML_customHeight
         // OOXTODO: XML_thickTop
         // OOXTODO: XML_thickBottom
diff --git a/sc/source/filter/inc/xetable.hxx b/sc/source/filter/inc/xetable.hxx
index 70fd2d14e2de..e8fa4f1eb28d 100644
--- a/sc/source/filter/inc/xetable.hxx
+++ b/sc/source/filter/inc/xetable.hxx
@@ -691,7 +691,7 @@ private:
     default width. If the passed width is rounded up or down to get the default
     width, the function returns false.
  */
-class XclExpDefcolwidth : public XclExpUInt16Record, protected XclExpRoot
+class XclExpDefcolwidth : public XclExpDoubleRecord, protected XclExpRoot
 {
 public:
     explicit            XclExpDefcolwidth( const XclExpRoot& rRoot );
@@ -701,6 +701,8 @@ public:
 
     /** Sets the passed column width (in 1/256 character width) as default width. */
     void                SetDefWidth( sal_uInt16 nXclColWidth );
+
+    virtual void        Save(XclExpStream& rStrm) override;
 };
 
 /** Contains the column settings for a range of columns.
@@ -732,8 +734,8 @@ public:
     /** Returns the number of columns represented by this record. */
     sal_uInt16   GetColCount() const { return mnLastXclCol - mnFirstXclCol + 1; }
 
-    /** Returns true, if the column has default format and width. */
-    bool                IsDefault( const XclExpDefcolwidth& rDefColWidth ) const;
+    /** Returns true, if the column has default format and width. Also sets mbCustomWidth */
+    bool                IsDefault( const XclExpDefcolwidth& rDefColWidth );
 
     virtual void        SaveXml( XclExpXmlStream& rStrm ) override;
 
@@ -775,6 +777,7 @@ public:
     virtual void        Save( XclExpStream& rStrm ) override;
     virtual void        SaveXml( XclExpXmlStream& rStrm ) override;
     sal_uInt8           GetHighestOutlineLevel() { return mnHighestOutlineLevel; }
+    double              GetDefColWidth() { return maDefcolwidth.GetValue(); }
 
 private:
     typedef XclExpRecordList< XclExpColinfo >   XclExpColinfoList;