Merge pull request #1077 from apetukhov/cellstyle_fix

Fix XSSFCellStyle.CloneStyleFrom
nissl-lab · Jun 14, 2023 · f8d21fc · f8d21fc
2 parents 2305df3 + a2f756c
commit f8d21fc
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 27 deletions.
diff --git a/OpenXmlFormats/Spreadsheet/Styles/CT_Xf.cs b/OpenXmlFormats/Spreadsheet/Styles/CT_Xf.cs
@@ -46,8 +46,12 @@ public class CT_Xf
 
         public CT_Xf Copy()
         {
-            CT_Xf obj = new CT_Xf();
-            if (this.alignment!=null)
+            return CopyTo(new CT_Xf());
+        }
+
+        public CT_Xf CopyTo(CT_Xf obj)
+        {
+            if (this.alignment != null)
                 obj.alignment = this.alignment.Copy();
             obj.protection = this.protection;
             obj.extLstField = null == extLstField ? null : this.extLstField.Copy();
@@ -69,6 +73,7 @@ public CT_Xf Copy()
             obj.pivotButtonField = this.pivotButtonField;
             obj.quotePrefixField = this.quotePrefixField;
             obj.xfIdField = this.xfIdField;
+            obj.xfIdSpecifiedField = this.xfIdSpecifiedField;
             return obj;
         }
 
@@ -187,6 +192,7 @@ public bool IsSetLocked()
             // first guess:
             return IsSetProtection() &&  (protectionField.locked == true);
         }
+
         public CT_CellProtection AddNewProtection()
         {
             this.protectionField = new CT_CellProtection();
@@ -295,6 +301,7 @@ public uint xfId
                 this.xfIdSpecifiedField = true;
             }
         }
+
         [XmlIgnore]
         public bool xfIdSpecified
         {

diff --git a/ooxml/XSSF/Model/StylesTable.cs b/ooxml/XSSF/Model/StylesTable.cs
@@ -41,7 +41,6 @@ namespace NPOI.XSSF.Model
     public class StylesTable : POIXMLDocumentPart
     {
         private SortedDictionary<short, String> numberFormats = new SortedDictionary<short, String>();
-        private bool[] usedNumberFormats = new bool[SpreadsheetVersion.EXCEL2007.MaxCellStyles];
         private List<XSSFFont> fonts = new List<XSSFFont>();
         private List<XSSFCellFill> fills = new List<XSSFCellFill>();
         private List<XSSFCellBorder> borders = new List<XSSFCellBorder>();
@@ -886,10 +885,13 @@ public XSSFCellStyle CreateCellStyle()
             ctXf.fontId = 0;
             ctXf.fillId = 0;
             ctXf.borderId = 0;
-            ctXf.xfId = 0;
+            if (xfSize > 0)
+            {
+                ctXf.xfId = 0; //default styleXf
+            }
 
             int indexXf = PutCellXf(ctXf);
-            return new XSSFCellStyle(indexXf - 1, xfSize - 1, this, theme);
+            return new XSSFCellStyle(indexXf - 1, ctXf.xfIdSpecified ? (int)ctXf.xfId : -1, this, theme);
         }
 
         /**

diff --git a/ooxml/XSSF/UserModel/XSSFCellStyle.cs b/ooxml/XSSF/UserModel/XSSFCellStyle.cs
@@ -130,8 +130,18 @@ public void CloneStyleFrom(ICellStyle source)
                 if (src._stylesSource == _stylesSource)
                 {
                     // Nice and easy
-                    _cellXf = src.GetCoreXf().Copy();
-                    _cellStyleXf = src.GetStyleXf() != null ? src.GetStyleXf().Copy() : null;
+                    src.GetCoreXf().CopyTo(_cellXf);
+
+                    // There is no need to copy _cellStyleXf couse XSSFCellStyle does not modify any _cellStyleXf properties.
+                    // Just update _cellStyleXf reference
+                    if (_cellXf.xfIdSpecified)
+                    {
+                        _cellStyleXf = _stylesSource.GetCellStyleXfAt((int)_cellXf.xfId);
+                    }
+                    else
+                    {
+                        _cellStyleXf = null;
+                    }
                 }
                 else
                 {
@@ -147,11 +157,6 @@ public void CloneStyleFrom(ICellStyle source)
 
                         // Create a new Xf with the same contents
                         _cellXf = src.GetCoreXf().Copy();
-                        if (_cellXf.applyBorder)
-                        {
-                            //CellXF is copied with existing Ids, but those are different if you're copying between two documents
-                            _cellXf.borderId = FindAddBorder(src._stylesSource.GetBorderAt((int)_cellXf.borderId).GetCTBorder());
-                        }
 
                         // bug 56295: ensure that the fills is available and set correctly
                         CT_Fill fill = CT_Fill.Parse(src.GetCTFill().ToString());
@@ -160,12 +165,7 @@ public void CloneStyleFrom(ICellStyle source)
                         // bug 58084: set borders correctly
                         CT_Border border = CT_Border.Parse(src.GetCTBorder().ToString());
                         AddBorder(border);
-
-                        if (src._cellStyleXf != null && src._cellStyleXf.applyBorder)
-                        {
-                            _cellStyleXf.borderId = FindAddBorder(src.GetCTBorder());
-                        }
-
+
                         // Swap it over
                         _stylesSource.ReplaceCellXfAt(_cellXfId, _cellXf);
                     }
@@ -659,7 +659,6 @@ public FillPattern FillPattern
             }
         }
 
-
         /**
         * Gets the font for this style
         * @return Font - font
@@ -1271,6 +1270,7 @@ public void SetBorderColor(BorderSide side, XSSFColor color)
                     break;
             }
         }
+
         private int FontId
         {
             get
@@ -1279,7 +1279,13 @@ private int FontId
                 {
                     return (int)_cellXf.fontId;
                 }
-                return _cellStyleXf != null ? (int)_cellStyleXf.fontId : -1;
+
+                if (_cellStyleXf != null && _cellStyleXf.IsSetFontId())
+                {
+                    return (int)_cellStyleXf.fontId;
+                }
+
+                return 0; //default font
             }
         }
 

diff --git a/testcases/ooxml/XSSF/UserModel/TestXSSFCellStyle.cs b/testcases/ooxml/XSSF/UserModel/TestXSSFCellStyle.cs
@@ -15,17 +15,16 @@ the License.  You may obtain a copy of the License at
    limitations under the License.
 ==================================================================== */
 
-using NPOI.XSSF.Model;
-using NPOI.OpenXmlFormats.Spreadsheet;
-using NPOI.XSSF.UserModel.Extensions;
-using NUnit.Framework;
-using NPOI.SS.UserModel;
 using NPOI.HSSF.UserModel;
 using NPOI.HSSF.Util;
-using NPOI.XSSF.UserModel;
+using NPOI.OpenXmlFormats.Spreadsheet;
+using NPOI.SS.UserModel;
 using NPOI.XSSF;
+using NPOI.XSSF.Model;
+using NPOI.XSSF.UserModel;
+using NPOI.XSSF.UserModel.Extensions;
+using NUnit.Framework;
 using SixLabors.ImageSharp;
-using SixLabors.ImageSharp.PixelFormats;
 
 namespace TestCases.XSSF.UserModel
 {
@@ -816,6 +815,7 @@ public void TestCloneStyleSameWB()
             Assert.AreEqual(fnt, clone.GetFont());
             Assert.AreEqual(18, clone.DataFormat);
             Assert.AreEqual(2, wb.NumberOfFonts);
+            Assert.AreEqual(clone.GetCoreXf(), wb.GetStylesSource().GetStyleAt(clone.Index).GetCoreXf(), "Should be same CoreXF after cloning");
 
             clone.Alignment = HorizontalAlignment.Left;
             clone.DataFormat = 17;