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 40d892a2. 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 555d7029 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/71132Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>

sc/qa/unit/subsequent_export-test.cxx

@@ -782,80 +782,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");
@@ -920,10 +900,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
@@ -1083,13 +1060,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);
 
     // First row is empty and default so it is not written into XML file
     // so we need to save 29 rows, as it provides information about outLineLevel

sc/source/filter/excel/xetable.cxx

@@ -1538,38 +1538,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,
@@ -1599,10 +1609,6 @@ XclExpColinfo::XclExpColinfo( const XclExpRoot& rRoot,
     // column flags
     ::set_flag( mnFlags, EXC_COLINFO_HIDDEN, rDoc.ColHidden(nScCol, nScTab) );
 
-    XclExpDefcolwidth defColWidth( 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() );
@@ -1615,12 +1621,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 )
@@ -2680,7 +2687,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

sc/source/filter/inc/xetable.hxx

@@ -676,7 +676,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 );
@@ -686,6 +686,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.
@@ -717,8 +719,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;
 
@@ -760,6 +762,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;