From 319e00adb1fef5b8b7c334c6b9e0f640649c642f Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Tue, 14 Oct 2025 14:53:20 +0800 Subject: [PATCH 1/7] Fix bug 61516: when copying cells with formulas we should properly check for references that are invalid afterwards. --- main/SS/Formula/FormulaShifter.cs | 10 ++- .../ooxml/XSSF/UserModel/TestXSSFBugs.cs | 63 ++++++++++++++++++- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/main/SS/Formula/FormulaShifter.cs b/main/SS/Formula/FormulaShifter.cs index d421de740..cab7fe93d 100644 --- a/main/SS/Formula/FormulaShifter.cs +++ b/main/SS/Formula/FormulaShifter.cs @@ -1005,13 +1005,19 @@ private Ptg RowCopyRefPtg(RefPtgBase rptg) int refRow = rptg.Row; if (rptg.IsRowRelative) { + // check new location where the ref is located int destRowIndex = _firstMovedIndex + _amountToMove; if (destRowIndex < 0 || _version.LastRowIndex < destRowIndex) { return CreateDeletedRef(rptg); } - - rptg.Row = refRow + _amountToMove; + // check new location where the ref points to + int newRowIndex = refRow + _amountToMove; + if(newRowIndex < 0 || _version.LastRowIndex < newRowIndex) + { + return CreateDeletedRef(rptg); + } + rptg.Row = newRowIndex; return rptg; } diff --git a/testcases/ooxml/XSSF/UserModel/TestXSSFBugs.cs b/testcases/ooxml/XSSF/UserModel/TestXSSFBugs.cs index 082a54e09..3c65c825b 100644 --- a/testcases/ooxml/XSSF/UserModel/TestXSSFBugs.cs +++ b/testcases/ooxml/XSSF/UserModel/TestXSSFBugs.cs @@ -19,9 +19,11 @@ limitations under the License. using NPOI.OpenXml4Net.OPC; using NPOI.OpenXmlFormats.Spreadsheet; using NPOI.POIFS.FileSystem; +using NPOI.SS; using NPOI.SS.Formula; using NPOI.SS.Formula.Eval; using NPOI.SS.Formula.Functions; +using NPOI.SS.Formula.PTG; using NPOI.SS.UserModel; using NPOI.SS.Util; using NPOI.Util; @@ -310,7 +312,7 @@ public void Bug48539() * a theme is used */ [Test] - public void Test48779() + public void Bug48779() { XSSFWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("48779.xlsx"); XSSFCell cell = wb.GetSheetAt(0).GetRow(0).GetCell(0) as XSSFCell; @@ -1567,7 +1569,7 @@ public void Test51710() * Bug 53101: */ [Test] - public void Test5301() + public void Bug5301() { IWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("53101.xlsx"); IFormulaEvaluator Evaluator = @@ -3484,7 +3486,7 @@ public void Test53611() // we currently only populate the dimension during writing out // to avoid having to iterate all rows/cells in each add/remove of a row or cell - IOUtils.Write(wb, new NullOutputStream()); + wb.Write(new NullOutputStream()); ClassicAssert.AreEqual("B2:I5", ((XSSFSheet)sheet).GetCTWorksheet().dimension.@ref); @@ -3509,6 +3511,59 @@ public void Bug61063() wb.Close(); } + [Test] + public void Bug61516() + { + string initialFormula = "A1"; + string expectedFormula = "#REF!"; // from ms excel + + IWorkbook wb = new XSSFWorkbook(); + ISheet sheet = wb.CreateSheet("sheet1"); + sheet.CreateRow(0).CreateCell(0).SetCellValue(1); // A1 = 1 + + { + ICell c3 = sheet.CreateRow(2).CreateCell(2); + c3.SetCellFormula(initialFormula); // C3 = =A1 + IFormulaEvaluator evaluator = wb.GetCreationHelper().CreateFormulaEvaluator(); + CellValue cellValue = evaluator.Evaluate(c3); + ClassicAssert.AreEqual(1, cellValue.NumberValue, 0.0001); + } + + { + FormulaShifter formulaShifter = FormulaShifter.CreateForRowCopy(0, "sheet1", 2/*firstRowToShift*/, 2/*lastRowToShift*/ + , -1/*step*/, SpreadsheetVersion.EXCEL2007); // parameters 2, 2, -1 should mean : Move row range [2-2] one level up + XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.Create((XSSFWorkbook) wb); + Ptg[] ptgs = FormulaParser.Parse(initialFormula, fpb, FormulaType.Cell, 0); // [A1] + formulaShifter.AdjustFormula(ptgs, 0); // adjusted to [A] + string shiftedFmla = FormulaRenderer.ToFormulaString(fpb, ptgs); //A + //Console.WriteLine(String.format("initial formula : A1; expected formula value After shifting up : #REF!; actual formula value : %s", shiftedFmla)); + ClassicAssert.AreEqual(expectedFormula, shiftedFmla, + "On copy we expect the formula to be adjusted, in this case it would point to row -1, which is an invalid REF"); + } + + { + FormulaShifter formulaShifter = FormulaShifter.CreateForRowShift(0, "sheet1", 2/*firstRowToShift*/, 2/*lastRowToShift*/ + , -1/*step*/, SpreadsheetVersion.EXCEL2007); // parameters 2, 2, -1 should mean : Move row range [2-2] one level up + XSSFEvaluationWorkbook fpb = XSSFEvaluationWorkbook.Create((XSSFWorkbook) wb); + Ptg[] ptgs = FormulaParser.Parse(initialFormula, fpb, FormulaType.Cell, 0); // [A1] + formulaShifter.AdjustFormula(ptgs, 0); // adjusted to [A] + string shiftedFmla = FormulaRenderer.ToFormulaString(fpb, ptgs); //A + //Console.WriteLine(String.format("initial formula : A1; expected formula value After shifting up : #REF!; actual formula value : %s", shiftedFmla)); + ClassicAssert.AreEqual(initialFormula, shiftedFmla, + "On Move we expect the formula to stay the same, thus expecting the initial formula A1 here"); + } + + sheet.ShiftRows(2, 2, -1); + { + ICell c2 = sheet.GetRow(1).GetCell(2); + ClassicAssert.IsNotNull(c2, "cell C2 needs to exist now"); + ClassicAssert.AreEqual(CellType.Formula, c2.CellType); + ClassicAssert.AreEqual(initialFormula, c2.CellFormula); + IFormulaEvaluator evaluator = wb.GetCreationHelper().CreateFormulaEvaluator(); + CellValue cellValue = evaluator.Evaluate(c2); + ClassicAssert.AreEqual(1, cellValue.NumberValue, 0.0001); + } + } [Test] public void TestBug690() { @@ -3588,5 +3643,7 @@ public void TestCopyEmptyRow() ClassicAssert.IsTrue(shiftedRow.GetCell(3).StringCellValue.Equals("D2")); } } + + } } From f5a4182a36470d9523f567f71c0b964939b0749d Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Tue, 14 Oct 2025 15:32:22 +0800 Subject: [PATCH 2/7] Add test which shows that bug 51262 is fixed now --- testcases/main/HSSF/UserModel/TestBugs.cs | 28 ++++++++++++++++++++++ testcases/test-data/spreadsheet/51262.xls | Bin 0 -> 25088 bytes 2 files changed, 28 insertions(+) create mode 100644 testcases/test-data/spreadsheet/51262.xls diff --git a/testcases/main/HSSF/UserModel/TestBugs.cs b/testcases/main/HSSF/UserModel/TestBugs.cs index ca2a09167..a29fb3fb9 100644 --- a/testcases/main/HSSF/UserModel/TestBugs.cs +++ b/testcases/main/HSSF/UserModel/TestBugs.cs @@ -3520,6 +3520,34 @@ public void Test61300() }); } + [Test] + public void Test51262() + { + HSSFWorkbook wb = HSSFTestDataSamples.OpenSampleWorkbook("51262.xls"); + ISheet sheet = wb.GetSheetAt(0); + IRow row = sheet.GetRow(2); + + ICell cell = row.GetCell(1); + ICellStyle style = cell.CellStyle; + ClassicAssert.AreEqual(26, style.FontIndex); + + row = sheet.GetRow(3); + cell = row.GetCell(1); + style = cell.CellStyle; + ClassicAssert.AreEqual(28, style.FontIndex); + + // check the two fonts + HSSFFont font = wb.GetFontAt((short) 26) as HSSFFont; + ClassicAssert.IsTrue(font.IsBold); + ClassicAssert.AreEqual(10, font.FontHeightInPoints); + ClassicAssert.AreEqual("\uFF2D\uFF33 \uFF30\u30B4\u30B7\u30C3\u30AF", font.FontName); + + font = wb.GetFontAt((short) 28) as HSSFFont; + ClassicAssert.IsTrue(font.IsBold); + ClassicAssert.AreEqual(10, font.FontHeightInPoints); + ClassicAssert.AreEqual("\uFF2D\uFF33 \uFF30\u30B4\u30B7\u30C3\u30AF", font.FontName); + } + // follow https://svn.apache.org/viewvc?view=revision&revision=1896552 to write a unit test for this fix. [Test] public void Test52447() diff --git a/testcases/test-data/spreadsheet/51262.xls b/testcases/test-data/spreadsheet/51262.xls new file mode 100644 index 0000000000000000000000000000000000000000..86ad84d4c1ae2803db57d8eb2f3671ba5fce2694 GIT binary patch literal 25088 zcmeHP33wbwm9Fj?NhA4^Z27*%k`LLI#2m6L#g?U+kz|`lmWh-~to_N&qteGXb*zvjKAeFac2p zmrVctn=@p~QIM7*8X=OPpjYJ7T9? zN=@94km-p(0i1%E3s3mGJ9zGKKj(xp||+4>Dw_o{oLtf^QjfiW$5V8XH~4OsI0E6 z^z^OY(pzO*^sZmwT!mp11~>)^_piSuGI&F3rV5O!Fak0%)p}Ct;F_e?L1h)zTo!72 z*3B3k`Gn}BC>z{{Wcp=ql|=Ne-=ZTbYAPUCUsX#VxYS~7h0#@CmATvox4F;;cS?z7 zQ7x`YWv102m7f&N#Im_L$+R8=Ij@PSQ9i-fC@fi9AZaR(s};^lah{`Oa}Em?nQgOpNVdKk8I4wSZ|T!6b$R*{ z2(~3JTKe2<{m;tQcbm(h6|_pod|K-#N;HdyW$T-mL5L)Iy=V#!EYXGu67}K{+4=@k zBX5Fl5|L8!8Tn`-#Bbw#YIuwA1yeTJ-#oj(2`ARsRY6 zr!wF_kpchd4ETF8;6IZA|H%ya_h-PrI|DwGJela9mvoAt7ODS)9zKu(ugR01Uy~;t ze{Tjl4`#qWlmY+c4ET3tz<)jiK3{o0E9vkdEmc37op3@y;_oagyKBZ>GlcVu%+H4@ z4L>yW*viLN3g;sdUx;Y|iT^z^$Yj{(+6?&f8SoF<;>$r((tk-OFEsj`)R6c@`)c;g zi4ci@5>JqFUPH^zNM*uf{zSub!bHcraf;#=yoE0(RAl~yel+{%gp5w-Mx3Ja)9CO) zRN@nI-mmLT$Db`LTQFk*^nAC(bK=IJujyISe<8;75^vEnCxs;5A}1$-B>qVe7GfLb zcQko8S!9==6GOk1Q%^XNNz2d4qMtgTrCW8hPKWaaGA_}OnDn^BQ;9d8esw&?V!(qA zPndK(C!*}=pvO+b-|8wCx3a>C)F!U^)$iVU)Kw#nrr?=D#pGDIoJhlar4HsLX)~%`h_&#-$vH z&@?j>VUWy$2+cJ!k@DP#(1bG+VRw*&BsA;HMA(;2OJu4ed+RVD=RhQT>oDr(KqPzX zuq(-dNcPrYzmo%z?5)GjB?lteTZbK1k_awR`x4D}eE#gM!*!M%h-7aaF4W{eBzx;{ zr6&g>*;~h*8)Suk?gHgl_0|L(pga$PZ@{NcX%WIwQt6gioL_qd96o|BH4K{@5@JpJ&_k* zd@(_!NGE*k*s-{=JmRv!6`OFNEmMgN?x@Qu6c<4?p*XN(vaQ*D_uZE!RC9B4nowq( zArx^MX|88!nlj_kN3k1`J?pr@I!><6w*c3C^Wwn8;k*iPMOiDr)@B2yC^mgq;z2ge zd07BYJE;W>8~f_7%{FEo+jtX1*qnqEuCyH@vj=(1{F=f@1|sq1>_q&z5V3YQ)^@D} zk)+nsBuO{tO(>A`vF7HR?Ufjx`o}bLzH&ZhZIt?iDr_C!aOikUNeGp09-C8$oQ-5K zkN7Xonuu7vy&<6<>N4$l(y+03d@4c22tnu0olEm}GtTgK;y_y_!`scc^fqfmUUQo@ z+qaorTr`IpLycyfA(T1X7($tG=^^MU2$i(FG)<)!;KF~hfYnr$qpoW$-N9BAaAtm% zq25Ww>}6*6HM)azNtzOp(A>}%srpPW(2^)%YZd7Q0@D>(Y^iGy1r%)XDG*SE)yYFy zso4~0&00V`tkdxPBA1LVeT zjRE{c1-;pu+bu|WK$3sdhy3k z+q0Q#W#dn0vo;GGEfz~Qg^{8E{>_i=+03)DsY_?GAqyKuYX{mHiYkf>J@?H2*t03O zve}f*rZx*3#(15L3vKhO|Fvf`-^!*wosBOG8!c+f+7w5I{`6l@*|S+dHg(x8Z6(;$ ziFaL z9T|F(pZ~q*f1t5pKtoEzwB=SdHkP(J3mdbg`TS4a^t!!*8tvU-Ev!(g`=RW*3dp2$>8yidWWMN~rG@t+U6MBTU zw#_Ol8yicj%fiNNX+HnU7f#yMW+rxt;K4l6Zw4&==JI$^#=cUBuH#>_Ct8^#3X8H5 zU1cF^wj!+g?C!KDx;jY|_G2Zw#zNF=GeX?+#n;;tU6UjVYq1htZy{>75FuiNAF(I8 zHc1q=VI^8)A!@b`8Sc!TqM9r-)#P80u+YxnPAr!;FV0iG9t1-3Z zm8Ee9cD07ZV4?>u;P4W&9vmIOP==5>qf z(G%Q|C{q)2qRd)L8E4#uGLzUXJub(-1Nu&Fa;c3Kp+v2vL{WTZT8UI%^Rb^EOEqnW z4~>LJ!_l$vBkrEa;pi#X_Eb{pChWXv!w%X*vh7D@`*Y*=5$L=J;v9xO@UhBheKxjk zUd5;zo{hGTjlod2q|#NE#GPWh-1?nrs+>GAXn~nQYc{qIVFGYRBs@4gy3dVg1&g4L zQfPJu1fuN^!#wvvK~RpGEBq~`>e#vY=~>|F9Y#- z^T#jzCn`eXelu~zKr8XNHpI0!n1Q%9G*Bh+Ix}&^LQ6n!%(EfRc&N2lky$OL)e^A@ z4Jmex9-4$HOr|!OT8{ecgk}%H7TM@Y_vd7d(P*5}xlH#j#Q+I$sbhHbKx9z&|En#+ zEJ9rBkPUqRjXszX0UQZyOvEB1`fvuP5Q>{AHbZ`~D>4}!4&) zDFznpiAKg#OyL;yJ&SoB-QZzTF#&hzi61&Et zkyEbzR2pgt{Dm~45Q^&qeCm;+U?Ru}X`IAK23qaP2x;1t5z@4))9MV-D6mY>H5rvH z{w534`5@gTb&%fX=8(?_nbPJ&BqhExb0T3aZXPq}~G#o z`*kqei;4??_rZRth(AJp&VUgL#Y}DYVYq+b@R2toFljc9kYuE^EX&if#4&Ebws6E| zs9p@X766Rm+WTp|QpFiac*b-m_?#l{0q!0Xjy1X+xZ6!QmcGli48Suj)eT&F}<@ea&j?!~0!EXYH5R?#kV`Iena z{PCZE^PSHA-OYUxzd_N~7I2<(-5_j7v}Cn{gow4C*A<4-4)xF3}?}@!hz56HDY@O3XNea(H3IlrWS` zNqQY)j5`5oU0RQlk}1y%C!(nnzN9KvZg~=VHT&P320WF0nd)R?;9S(^bb^-HgJeq| z#>`VO`YA>JsfgXxieIMhHXBDpoW(Qj*<{C*MsTYfNf3Q4e8MG;%HA56#Q~X@*W~cb z!T^V-@;(9~IuGr>Q2D~9_P)9zg?Fi?!0>#wtYb_G2;$hYSaD7p90$}6@q*-j0cHP$ zaRli#ON^ox;z|`Q5>J{z00aV_)I#!%G+MIJI_ULATT-Vq zEsDvJEO#+;ERB^7*t7Qi2Y!E{4CiwJ^8m(Ja{;$|Pa5bOg*a_W<_l*MEoO^?+Xe`VIQg+-`rVb7cVxfdTI;u!1LXggr@v`ga^x z>P|vic}ZvDnL9PnCCQBDXLe@=kBJ(G!#MIhYtqzR6vzm(qN@hZrsitY|geTwKVO`P4So#Fmg?aWZMf+uBj+ZETm(Tca2;;*5RrCsMIETCFhu8FC zdMa@{h`^Z^OXNi&FYjx2d6t^8;m!BV=cyEoE9Ma5C)(ka{a*SvnlcHdS{yN`*aHuU zKr}i&+&_sYQ}=bjzFpyiIB9ZM;8PUB_!dR!fr?orCASB|6N-Af=N6C0;9bnM`8 z;hUYU4#vb>QXc+)_?^_ytNu@$xB}@)`9GsT(!Hgrw_qBS zj{khn8J?n-c~RoRK?2}fAwNK zWbSq9FyGvM8CF+`-lrCJTZz7_5@WBcz(M=|(sqod0^%*`)p~*N5}o+#La(P`Ix!v# zAx9U+TN+0Dep0Xg6{mWXBBl*}TP?chvkg@1nSl)<>vhf zz!09A+|b1k{wR3_Im}mHj1YREi@l)S22Iq;^EPm@qr>H0(x8r5lI=t4{o*EI2P7}9 z3G_?JtltTH)YeKkOKgrv#`vQK5^3kjyT-lb*rZaI!mOP=uz+1?*)EK*yHUFdtY=Kf z`Qn3U3D%cagU}9_d{_%E{`8~HUC=-qY7~$wSzM=2wzRpSs}k+(1D|$`uv#H&9VD*7 z(?t-N0M5L?d2v+(Xa!x)cvJEK{3APTSz1l%aLlcpjlBWfU@^S~@EMwGM_kdN z1+?Rje$pn}kyg_UY|THkCpucZAsFQOt|IPus}sA9ew#}XV%RjsR}hB=fveey`x z?3D4LU015e+;(>~9PeAxOR9<|&P?+e<;`{MTvJdb?tiUGLRHwSMI z{2mWv1o2}VKlS^Hj`4P9c;NiGQsHF2`cGA2 znbEm>eC(#kK-62ZuoNExoQMvNjYLl0SSm`GN5A>y$9S)6XlQsKGJmpE%ygQ+#&_$_ zOGPoHiaQk1*LHJwWD*0FMNhGaHfQ<#5@c}l<9!ocKfZX0e2s7U9ax0vTlxguZay7F zZ-vE(B*r-=+<=9%PuoJXkqOVTG6Kz29V{j1t`s@bMCzBHl$Rfjah$|KlxKsgo-7uJ z$_sZm#0jk9KI?R1@@$zfw0GM;v;e_Ew5a7b6{;moYDtkgu9Uk93g;H9b7g3}Q_NIf zyHzb&Ae8Z#)&kqq#feTW#$Ig};{6)+4Z^XZcRQXkV|dK0^JB2-TJCg)5OrrObMcTV z>W^0XYCV>Y zt-;=|_V%6Vu)MzB&hD;SPisd<<=>aSwbSh_i@nj^)qnh=yJPsq{_*hm5zNFXbIMLU zsfu|g951Phl@lM=4N%DML2eg|3S#qmV)x5g?Lh3j{JQ7`W$fAu%93x+@cnScvI}!D zRkN~*#7v^?v~0Ek(RxjpkA`0 z=%0MMihjQI@H@Y;v^2mKCKfhz$lSBBq)RSMpJ(B zG`s@+zaG4Quk#-{6dD_iV$UZH(F2pXE;2rWEx>TrLx=|bBN4x^slwmr_xXIVF=PmJ zZf~k+gnc&ngWiVCW}u1g4mMTPZK~V6sXf#h*u1$R(4YbCZ4Wh7w0qnAZ9z|{EktPd z13jzc`W<5kN^OUuk+x&~9NltVux(4Clt)sQJq;$ Date: Tue, 14 Oct 2025 16:42:28 +0800 Subject: [PATCH 3/7] Remove "filling" in IntList as this has no effect whatsoever as far as I could see --- main/Util/IntList.cs | 37 ++------------ testcases/main/Util/TestIntList.cs | 81 +++++++++++++++++++----------- 2 files changed, 57 insertions(+), 61 deletions(-) diff --git a/main/Util/IntList.cs b/main/Util/IntList.cs index 00b4dc455..ff783ff50 100644 --- a/main/Util/IntList.cs +++ b/main/Util/IntList.cs @@ -46,7 +46,7 @@ public class IntList { private int[] _array; private int _limit; - private int fillval = 0; + private static int _default_size = 128; /// @@ -57,10 +57,10 @@ public IntList() { } - public IntList(int InitialCapacity) - : this(InitialCapacity, 0) + public IntList(int initialCapacity) { - + _array = new int[initialCapacity]; + _limit = 0; } /// @@ -74,30 +74,6 @@ public IntList(IntList list) _limit = list._limit; } - /// - /// create an IntList with a predefined Initial size - /// - /// the size for the internal array - /// - public IntList(int initialCapacity, int fillvalue) - { - _array = new int[initialCapacity]; - if (fillval != 0) - { - fillval = fillvalue; - FillArray(fillval, _array, 0); - } - _limit = 0; - } - - private static void FillArray(int val, int[] array, int index) - { - for (int k = index; k < array.Length; k++) - { - array[k] = val; - } - } - /// /// add the specfied value at the specified index /// @@ -580,11 +556,6 @@ private void growArray(int new_size) : new_size; int[] new_array = new int[size]; - if (fillval != 0) - { - FillArray(fillval, new_array, _array.Length); - } - Array.Copy(_array, 0, new_array, 0, _limit); _array = new_array; } diff --git a/testcases/main/Util/TestIntList.cs b/testcases/main/Util/TestIntList.cs index e0420aebd..bebc31f77 100644 --- a/testcases/main/Util/TestIntList.cs +++ b/testcases/main/Util/TestIntList.cs @@ -201,6 +201,18 @@ public void TestAddAll() ClassicAssert.AreEqual(list.Get(4), empty.Get(9)); ClassicAssert.AreEqual(list.Get(4), empty.Get(14)); } + + [Test] + public void AestAddAllGrow() + { + IntList list = new IntList(0); + IntList addList = new IntList(0); + addList.Add(1); + addList.Add(2); + + ClassicAssert.IsTrue(list.AddAll(0, addList)); + } + [Test] public void TestClear() { @@ -240,7 +252,7 @@ public void TestContains() } else { - ClassicAssert.IsTrue(!list.Contains(j)); + ClassicAssert.IsFalse(list.Contains(j)); } } } @@ -260,10 +272,10 @@ public void TestContainsAll() ClassicAssert.IsTrue(list.ContainsAll(list2)); list2.Add(10); ClassicAssert.IsTrue(list2.ContainsAll(list)); - ClassicAssert.IsTrue(!list.ContainsAll(list2)); + ClassicAssert.IsFalse(list.ContainsAll(list2)); list.Add(11); - ClassicAssert.IsTrue(!list2.ContainsAll(list)); - ClassicAssert.IsTrue(!list.ContainsAll(list2)); + ClassicAssert.IsFalse(list2.ContainsAll(list)); + ClassicAssert.IsFalse(list.ContainsAll(list2)); } [Test] public void TestEquals() @@ -271,7 +283,7 @@ public void TestEquals() IntList list = new IntList(); ClassicAssert.AreEqual(list, list); - ClassicAssert.IsTrue(!list.Equals(null)); + ClassicAssert.IsFalse(list.Equals(null)); IntList list2 = new IntList(200); ClassicAssert.IsTrue(list.Equals(list2));//ClassicAssert.AreEqual(list, list2); @@ -281,14 +293,15 @@ public void TestEquals() list.Add(1); list2.Add(1); list2.Add(0); - ClassicAssert.IsTrue(!list.Equals(list2)); + ClassicAssert.IsFalse(list.Equals(list2)); list2.RemoveValue(1); list2.Add(1); ClassicAssert.IsTrue(list.Equals(list2));//ClassicAssert.AreEqual(list, list2); ClassicAssert.IsTrue(list2.Equals(list));//ClassicAssert.AreEqual(list2, list); + ClassicAssert.AreEqual(list.GetHashCode(), list.GetHashCode()); list2.Add(2); - ClassicAssert.IsTrue(!list.Equals(list2)); - ClassicAssert.IsTrue(!list2.Equals(list)); + ClassicAssert.IsFalse(list.Equals(list2)); + ClassicAssert.IsFalse(list2.Equals(list)); } [Test] public void TestGet() @@ -352,9 +365,9 @@ public void TestIsEmpty() list1.Add(1); list2.Add(2); list3 = new IntList(list2); - ClassicAssert.IsTrue(!list1.IsEmpty()); - ClassicAssert.IsTrue(!list2.IsEmpty()); - ClassicAssert.IsTrue(!list3.IsEmpty()); + ClassicAssert.IsFalse(list1.IsEmpty()); + ClassicAssert.IsFalse(list2.IsEmpty()); + ClassicAssert.IsFalse(list3.IsEmpty()); list1.Clear(); list2.Remove(0); list3.RemoveValue(2); @@ -433,7 +446,7 @@ public void TestRemoveValue() ClassicAssert.IsTrue(list.RemoveValue(j)); ClassicAssert.IsTrue(list.RemoveValue(j)); } - ClassicAssert.IsTrue(!list.RemoveValue(j)); + ClassicAssert.IsFalse(list.RemoveValue(j)); } } [Test] @@ -460,16 +473,22 @@ public void TestRemoveAll() listOdd.Add(j); } } - list.RemoveAll(listEven); - //ClassicAssert.AreEqual(list, listOdd); - ClassicAssert.IsTrue(list.Equals(listOdd)); - list.RemoveAll(listOdd); + ClassicAssert.IsTrue(list.RemoveAll(listEven)); + ClassicAssert.AreEqual(list, listOdd); + + ClassicAssert.IsTrue(list.RemoveAll(listOdd)); ClassicAssert.IsTrue(list.IsEmpty()); - listCopy.RemoveAll(listOdd); - //ClassicAssert.AreEqual(listCopy, listEven); - ClassicAssert.IsTrue(listCopy.Equals(listEven)); - listCopy.RemoveAll(listEven); + + ClassicAssert.IsTrue(listCopy.RemoveAll(listOdd)); + ClassicAssert.AreEqual(listCopy, listEven); + + ClassicAssert.IsTrue(listCopy.RemoveAll(listEven)); ClassicAssert.IsTrue(listCopy.IsEmpty()); + + ClassicAssert.IsFalse(list.RemoveAll(listEven)); + ClassicAssert.IsFalse(list.RemoveAll(listOdd)); + ClassicAssert.IsFalse(listCopy.RemoveAll(listEven)); + ClassicAssert.IsFalse(listCopy.RemoveAll(listEven)); } [Test] public void TestRetainAll() @@ -495,16 +514,22 @@ public void TestRetainAll() listOdd.Add(j); } } - list.RetainAll(listOdd); - //ClassicAssert.AreEqual(list, listOdd); - ClassicAssert.IsTrue(list.Equals(listOdd)); - list.RetainAll(listEven); + ClassicAssert.IsTrue(list.RetainAll(listOdd)); + ClassicAssert.AreEqual(list, listOdd); + + ClassicAssert.IsTrue(list.RetainAll(listEven)); ClassicAssert.IsTrue(list.IsEmpty()); - listCopy.RetainAll(listEven); - //ClassicAssert.AreEqual(listCopy, listEven); - ClassicAssert.IsTrue(listCopy.Equals(listEven)); - listCopy.RetainAll(listOdd); + + ClassicAssert.IsTrue(listCopy.RetainAll(listEven)); + ClassicAssert.AreEqual(listCopy, listEven); + + ClassicAssert.IsTrue(listCopy.RetainAll(listOdd)); ClassicAssert.IsTrue(listCopy.IsEmpty()); + + ClassicAssert.IsFalse(list.RetainAll(listOdd)); + ClassicAssert.IsFalse(list.RetainAll(listEven)); + ClassicAssert.IsFalse(listCopy.RetainAll(listEven)); + ClassicAssert.IsFalse(listCopy.RetainAll(listOdd)); } [Test] public void TestSet() From 5125b73d19d1b0d5a68c57e3ee59d649e662eaa3 Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Wed, 15 Oct 2025 08:46:51 +0800 Subject: [PATCH 4/7] Bug 58068: Add a method to pass the actual Color to StylesTable.findFont(). --- ooxml/XSSF/Model/StylesTable.cs | 26 +++- ooxml/XSSF/UserModel/XSSFFont.cs | 8 ++ .../ooxml/XSSF/UserModel/TestXSSFFont.cs | 111 +++++++++++++++++- 3 files changed, 143 insertions(+), 2 deletions(-) diff --git a/ooxml/XSSF/Model/StylesTable.cs b/ooxml/XSSF/Model/StylesTable.cs index 72f123ea8..1afeedf2e 100644 --- a/ooxml/XSSF/Model/StylesTable.cs +++ b/ooxml/XSSF/Model/StylesTable.cs @@ -926,7 +926,8 @@ public XSSFCellStyle CreateCellStyle() /** * Finds a font that matches the one with the supplied attributes */ - public XSSFFont FindFont(bool bold, short color, short fontHeight, String name, bool italic, bool strikeout, FontSuperScript typeOffset, FontUnderlineType underline) + public XSSFFont FindFont(bool bold, short color, short fontHeight, String name, bool italic, bool strikeout, + FontSuperScript typeOffset, FontUnderlineType underline) { foreach (XSSFFont font in fonts) { @@ -945,6 +946,29 @@ public XSSFFont FindFont(bool bold, short color, short fontHeight, String name, return null; } + /// + /// Finds a font that matches the one with the supplied attributes, + /// where color is the actual Color-value, not the indexed color + /// + public XSSFFont FindFont(bool bold, IColor color, short fontHeight, string name, bool italic, bool strikeout, + FontSuperScript typeOffset, FontUnderlineType underline) + { + foreach(XSSFFont font in fonts) + { + if((font.IsBold == bold) + && font.GetXSSFColor().Equals(color) + && font.FontHeight == fontHeight + && font.FontName.Equals(name) + && font.IsItalic == italic + && font.IsStrikeout == strikeout + && font.TypeOffset == typeOffset + && font.Underline == underline) + { + return font; + } + } + return null; + } /// /// default or custom indexed color to RGB mapping /// diff --git a/ooxml/XSSF/UserModel/XSSFFont.cs b/ooxml/XSSF/UserModel/XSSFFont.cs index 5e9eb65df..b3cd2341c 100644 --- a/ooxml/XSSF/UserModel/XSSFFont.cs +++ b/ooxml/XSSF/UserModel/XSSFFont.cs @@ -629,6 +629,7 @@ public void SetFamily(FontFamily family) * @return unique index number of the underlying record this Font represents (probably you don't care * unless you're comparing which one is which) */ + [Obsolete] public short Index { get @@ -636,6 +637,13 @@ public short Index return _index; } } + public int IndexAsInt + { + get + { + return _index; + } + } public override int GetHashCode() { diff --git a/testcases/ooxml/XSSF/UserModel/TestXSSFFont.cs b/testcases/ooxml/XSSF/UserModel/TestXSSFFont.cs index 5d398bf67..0fb5bb334 100644 --- a/testcases/ooxml/XSSF/UserModel/TestXSSFFont.cs +++ b/testcases/ooxml/XSSF/UserModel/TestXSSFFont.cs @@ -16,13 +16,15 @@ limitations under the License. ==================================================================== */ using NPOI; +using NPOI.OOXML.XSSF.UserModel; using NPOI.OpenXmlFormats.Spreadsheet; using NPOI.SS.UserModel; using NPOI.SS.Util; using NPOI.Util; using NPOI.XSSF; using NPOI.XSSF.UserModel; -using NUnit.Framework;using NUnit.Framework.Legacy; +using NUnit.Framework; +using NUnit.Framework.Legacy; using System.Text; using TestCases.SS.UserModel; @@ -323,5 +325,112 @@ public void TestCanComputeWidthInvalidFont() // Even with invalid fonts we still get back useful data most of the time... SheetUtil.CanComputeColumnWidth(font); } + + /// + /// Test that fonts Get added properly + /// + [Test] + public void TestFindFont() + { + XSSFWorkbook wb = new XSSFWorkbook(); + ClassicAssert.AreEqual(1, wb.NumberOfFonts); + + XSSFSheet s = wb.CreateSheet() as XSSFSheet; + s.CreateRow(0); + s.CreateRow(1); + s.GetRow(0).CreateCell(0); + s.GetRow(1).CreateCell(0); + + ClassicAssert.AreEqual(1, wb.NumberOfFonts); + + XSSFFont f1 = wb.GetFontAt(0) as XSSFFont; + ClassicAssert.IsFalse(f1.IsBold); + + // Check that asking for the same font + // multiple times gives you the same thing. + // Otherwise, our tests wouldn't work! + ClassicAssert.AreSame(wb.GetFontAt(0), wb.GetFontAt(0)); + ClassicAssert.AreEqual( + wb.GetFontAt(0), + wb.GetFontAt(0) + ); + + // Look for a new font we have + // yet to add + ClassicAssert.IsNull( + wb.FindFont( + false, IndexedColors.Indigo.Index, (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + ClassicAssert.IsNull( + wb.GetStylesSource().FindFont( + false, new XSSFColor(IndexedColors.Indigo, new DefaultIndexedColorMap()), (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + + XSSFFont nf = wb.CreateFont() as XSSFFont; + ClassicAssert.AreEqual(2, wb.NumberOfFonts); + + ClassicAssert.AreEqual(1, nf.IndexAsInt); + ClassicAssert.AreEqual(nf, wb.GetFontAt(1)); + + nf.IsBold = false; + nf.Color = IndexedColors.Indigo.Index; + nf.FontHeight = 22; + nf.FontName = "Thingy"; + nf.IsItalic = false; + nf.IsStrikeout = true; + nf.TypeOffset = (FontSuperScript) 2; + nf.Underline = (FontUnderlineType) 2; + + ClassicAssert.AreEqual(2, wb.NumberOfFonts); + ClassicAssert.AreEqual(nf, wb.GetFontAt(1)); + + ClassicAssert.IsTrue( + wb.GetFontAt(0) + != + wb.GetFontAt(1) + ); + + // Find it now + ClassicAssert.IsNotNull( + wb.FindFont( + false, IndexedColors.Indigo.Index, (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + ClassicAssert.IsNotNull( + wb.GetStylesSource().FindFont( + false, new XSSFColor(IndexedColors.Indigo, new DefaultIndexedColorMap()), (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + + XSSFFont font = wb.FindFont( + false, IndexedColors.Indigo.Index, (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) as XSSFFont; + ClassicAssert.IsNotNull(font); + ClassicAssert.AreEqual( + 1, + font.IndexAsInt + ); + ClassicAssert.AreEqual(nf, + wb.FindFont( + false, IndexedColors.Indigo.Index, (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + ClassicAssert.AreEqual(nf, + wb.GetStylesSource().FindFont( + false, new XSSFColor(IndexedColors.Indigo, new DefaultIndexedColorMap()), (short) 22, + "Thingy", false, true, (FontSuperScript) 2, (FontUnderlineType) 2 + ) + ); + + wb.Close(); + } } } \ No newline at end of file From 2ff32f330579625ff712861e61f7dcc8cfd32e05 Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Wed, 15 Oct 2025 08:48:37 +0800 Subject: [PATCH 5/7] Fix aparent copy/paste error in XSSFBorderFormatting --- ooxml/XSSF/UserModel/XSSFBorderFormatting.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooxml/XSSF/UserModel/XSSFBorderFormatting.cs b/ooxml/XSSF/UserModel/XSSFBorderFormatting.cs index 481a33b74..49b2253dd 100644 --- a/ooxml/XSSF/UserModel/XSSFBorderFormatting.cs +++ b/ooxml/XSSF/UserModel/XSSFBorderFormatting.cs @@ -171,7 +171,7 @@ public short TopBorderColor { get { - return GetIndexedColor(RightBorderColorColor as XSSFColor); + return GetIndexedColor(TopBorderColorColor as XSSFColor); } set { From a84ed5f44f27aa826a57ae805e3e2abd0ea6f82d Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Wed, 15 Oct 2025 09:00:19 +0800 Subject: [PATCH 6/7] simplfy PackagingURIHelper#combine boolean logic using xor and replacing +FORWARD_SLASH_CHAR with FORWARD_SLASH_STRING --- openxml4Net/OPC/PackagingUriHelper.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/openxml4Net/OPC/PackagingUriHelper.cs b/openxml4Net/OPC/PackagingUriHelper.cs index 361b9c036..cf3c9671d 100644 --- a/openxml4Net/OPC/PackagingUriHelper.cs +++ b/openxml4Net/OPC/PackagingUriHelper.cs @@ -262,13 +262,9 @@ public static Uri Combine(Uri prefix, Uri suffix) */ public static String Combine(String prefix, String suffix) { - if (!prefix.EndsWith("" + FORWARD_SLASH_CHAR) - && !suffix.StartsWith("" + FORWARD_SLASH_CHAR)) + if (!prefix.EndsWith(FORWARD_SLASH_STRING) && !suffix.StartsWith(FORWARD_SLASH_STRING)) return prefix + FORWARD_SLASH_CHAR + suffix; - else if ((!prefix.EndsWith("" + FORWARD_SLASH_CHAR) - && suffix.StartsWith("" + FORWARD_SLASH_CHAR) || (prefix - .EndsWith("" + FORWARD_SLASH_CHAR) && !suffix.StartsWith("" - + FORWARD_SLASH_CHAR)))) + else if (prefix.EndsWith(FORWARD_SLASH_STRING) ^ suffix.StartsWith(FORWARD_SLASH_STRING)) return prefix + suffix; else return ""; From 12da656b4f42c412345518236c4b6ad1b0aff408 Mon Sep 17 00:00:00 2001 From: Antony Liu Date: Wed, 15 Oct 2025 09:26:16 +0800 Subject: [PATCH 7/7] bug 61630: performance improvements in XSSFExportToXml. Thanks to Daniel for the patch. --- main/SS/UserModel/FractionFormat.cs | 2 +- ooxml/XSSF/Extractor/XSSFExportToXml.cs | 71 ++++++++++++------------- 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/main/SS/UserModel/FractionFormat.cs b/main/SS/UserModel/FractionFormat.cs index c2bbc2a6c..db8e609df 100644 --- a/main/SS/UserModel/FractionFormat.cs +++ b/main/SS/UserModel/FractionFormat.cs @@ -173,7 +173,7 @@ public String Format(string num) } //if whole part has to go into the numerator - if ("".Equals(wholePartFormatString)) + if (string.IsNullOrEmpty(wholePartFormatString)) { int trueNum = (fract.Denominator * (int)wholePart) + fract.Numerator; sb1.Append(trueNum).Append("/").Append(fract.Denominator); diff --git a/ooxml/XSSF/Extractor/XSSFExportToXml.cs b/ooxml/XSSF/Extractor/XSSFExportToXml.cs index 9f9332c17..7e05fcc2b 100644 --- a/ooxml/XSSF/Extractor/XSSFExportToXml.cs +++ b/ooxml/XSSF/Extractor/XSSFExportToXml.cs @@ -494,38 +494,38 @@ private static int IndexOfElementInComplexType(String elementName, XmlNode compl { return -1; } - XmlNodeList list = complexType.ChildNodes; + int indexOf = -1; + int i = 0; + XmlNode node = complexType.FirstChild; + String elementNameWithoutNamespace = RemoveNamespace(elementName); - for (int i = 0; i < list.Count; i++) + while(node != null) { - XmlNode node = list[i]; - if (node is XmlElement) - { - if (node.LocalName.Equals("element")) + if(node is XmlElement && "element".Equals(node.LocalName)) { + XmlNode element = GetNameOrRefElement(node); + if(element.Value.Equals(elementNameWithoutNamespace)) { - XmlNode element = GetNameOrRefElement(node); - if (element.Value.Equals(RemoveNamespace(elementName))) - { - indexOf = i; - break; - } - + indexOf = i; + break; } } + i++; + node = node.NextSibling; } + return indexOf; } private static XmlNode GetNameOrRefElement(XmlNode node) { - XmlNode returnNode = node.Attributes.GetNamedItem("name"); + XmlNode returnNode = node.Attributes.GetNamedItem("ref"); if(returnNode != null) { return returnNode; } - return node.Attributes.GetNamedItem("ref"); + return node.Attributes.GetNamedItem("name"); } private static XmlNode GetComplexTypeForElement(String elementName, XmlNode xmlSchema, XmlNode localComplexTypeRootNode) @@ -549,37 +549,33 @@ private static String GetComplexTypeNameFromChildren(XmlNode localComplexTypeRoo { return ""; } - XmlNodeList list = localComplexTypeRootNode.ChildNodes; + XmlNode node = localComplexTypeRootNode.FirstChild; String complexTypeName = ""; - for(int i = 0; i < list.Count; i++) + while(node != null) { - XmlNode node = list[i]; - if(node is XmlElement) + if(node is XmlElement && "element".Equals(node.LocalName)) { - if(node.LocalName.Equals("element")) + XmlNode nameAttribute = node.Attributes.GetNamedItem("name"); + if(nameAttribute.Value.Equals(elementNameWithoutNamespace)) { - XmlNode nameAttribute = node.Attributes.GetNamedItem("name"); - if(nameAttribute.Value.Equals(elementNameWithoutNamespace)) + XmlNode complexTypeAttribute = node.Attributes.GetNamedItem("type"); + if(complexTypeAttribute != null) { - XmlNode complexTypeAttribute = node.Attributes.GetNamedItem("type"); - if(complexTypeAttribute != null) - { - complexTypeName = complexTypeAttribute.Value; - break; - } + complexTypeName = complexTypeAttribute.Value; + break; } } } + node = node.NextSibling; } return complexTypeName; } private static XmlNode GetComplexTypeNodeFromSchemaChildren(XmlNode xmlSchema, XmlNode complexTypeNode, String complexTypeName) { - XmlNodeList complexTypeList = xmlSchema.ChildNodes; - for(int i = 0; i < complexTypeList.Count; i++) + XmlNode node = xmlSchema.FirstChild; + while(node != null) { - XmlNode node = complexTypeList[i]; if(node is XmlElement) { if(node.LocalName.Equals("complexType")) @@ -587,29 +583,28 @@ private static XmlNode GetComplexTypeNodeFromSchemaChildren(XmlNode xmlSchema, X XmlNode nameAttribute = node.Attributes.GetNamedItem("name"); if(nameAttribute.Value.Equals(complexTypeName)) { - - XmlNodeList complexTypeChildList = node.ChildNodes; - for(int j = 0; j < complexTypeChildList.Count; j++) + XmlNode sequence = node.FirstChild; + while(sequence != null) { - XmlNode sequence = complexTypeChildList[j]; - if(sequence is XmlElement) { - if(sequence.LocalName.Equals("sequence")) + String localName = sequence.LocalName; + if("sequence".Equals(localName) || "all".Equals(localName)) { complexTypeNode = sequence; break; } } + sequence = sequence.NextSibling; } if(complexTypeNode != null) { break; } - } } } + node = node.NextSibling; } return complexTypeNode; }