From 30d20582af1ae32a428424300f23e1345bff42c3 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 14 Mar 2015 19:32:28 +0000 Subject: [PATCH] Bug 56295: Fix cloning of styles across workbooks and handling of default value of attribute applyFill git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1666736 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/xssf/usermodel/XSSFCellStyle.java | 41 ++++++++----- .../poi/xssf/usermodel/TestXSSFBugs.java | 57 ++++++++++++++++++ .../poi/xssf/usermodel/TestXSSFCellStyle.java | 10 ++- test-data/spreadsheet/56295.xlsx | Bin 0 -> 4773 bytes 4 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 test-data/spreadsheet/56295.xlsx diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java index d1f075907..760026be1 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCellStyle.java @@ -154,6 +154,13 @@ public class XSSFCellStyle implements CellStyle { _cellXf = CTXf.Factory.parse( src.getCoreXf().toString() ); + + // bug 56295: ensure that the fills is available and set correctly + CTFill fill = CTFill.Factory.parse( + src.getCTFill().toString() + ); + addFill(fill); + // Swap it over _stylesSource.replaceCellXfAt(_cellXfId, _cellXf); } catch(XmlException e) { @@ -187,6 +194,13 @@ public class XSSFCellStyle implements CellStyle { } } + private void addFill(CTFill fill) { + int idx = _stylesSource.putFill(new XSSFCellFill(fill)); + + _cellXf.setFillId(idx); + _cellXf.setApplyFill(true); + } + /** * Get the type of horizontal alignment for the cell * @@ -444,7 +458,8 @@ public class XSSFCellStyle implements CellStyle { * @return XSSFColor - fill color or null if not set */ public XSSFColor getFillBackgroundXSSFColor() { - if(!_cellXf.getApplyFill()) return null; + // bug 56295: handle missing applyFill attribute as "true" because Excel does as well + if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return null; int fillIndex = (int)_cellXf.getFillId(); XSSFCellFill fg = _stylesSource.getFillAt(fillIndex); @@ -480,7 +495,8 @@ public class XSSFCellStyle implements CellStyle { * @return XSSFColor - fill color or null if not set */ public XSSFColor getFillForegroundXSSFColor() { - if(!_cellXf.getApplyFill()) return null; + // bug 56295: handle missing applyFill attribute as "true" because Excel does as well + if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return null; int fillIndex = (int)_cellXf.getFillId(); XSSFCellFill fg = _stylesSource.getFillAt(fillIndex); @@ -515,7 +531,8 @@ public class XSSFCellStyle implements CellStyle { * @see org.apache.poi.ss.usermodel.CellStyle#DIAMONDS */ public short getFillPattern() { - if(!_cellXf.getApplyFill()) return 0; + // bug 56295: handle missing applyFill attribute as "true" because Excel does as well + if(_cellXf.isSetApplyFill() && !_cellXf.getApplyFill()) return 0; int fillIndex = (int)_cellXf.getFillId(); XSSFCellFill fill = _stylesSource.getFillAt(fillIndex); @@ -996,10 +1013,7 @@ public class XSSFCellStyle implements CellStyle { ptrn.setBgColor(color.getCTColor()); } - int idx = _stylesSource.putFill(new XSSFCellFill(ct)); - - _cellXf.setFillId(idx); - _cellXf.setApplyFill(true); + addFill(ct); } /** @@ -1052,10 +1066,7 @@ public class XSSFCellStyle implements CellStyle { ptrn.setFgColor(color.getCTColor()); } - int idx = _stylesSource.putFill(new XSSFCellFill(ct)); - - _cellXf.setFillId(idx); - _cellXf.setApplyFill(true); + addFill(ct); } /** @@ -1076,7 +1087,8 @@ public class XSSFCellStyle implements CellStyle { */ private CTFill getCTFill(){ CTFill ct; - if(_cellXf.getApplyFill()) { + // bug 56295: handle missing applyFill attribute as "true" because Excel does as well + if(!_cellXf.isSetApplyFill() || _cellXf.getApplyFill()) { int fillIndex = (int)_cellXf.getFillId(); XSSFCellFill cf = _stylesSource.getFillAt(fillIndex); @@ -1135,10 +1147,7 @@ public class XSSFCellStyle implements CellStyle { if(fp == NO_FILL && ptrn.isSetPatternType()) ptrn.unsetPatternType(); else ptrn.setPatternType(STPatternType.Enum.forInt(fp + 1)); - int idx = _stylesSource.putFill(new XSSFCellFill(ct)); - - _cellXf.setFillId(idx); - _cellXf.setApplyFill(true); + addFill(ct); } /** diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index bb9efa407..5caf72988 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -31,6 +31,7 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.util.Calendar; import java.util.List; @@ -2308,4 +2309,60 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { // TODO Re-check sheet contents // TODO Re-check formula evaluation } + + @Test + public void testBug56295_MergeXlslsWithStyles() throws IOException { + XSSFWorkbook xlsToAppendWorkbook = XSSFTestDataSamples.openSampleWorkbook("56295.xlsx"); + XSSFSheet sheet = xlsToAppendWorkbook.getSheetAt(0); + XSSFRow srcRow = sheet.getRow(0); + XSSFCell oldCell = srcRow.getCell(0); + XSSFCellStyle cellStyle = oldCell.getCellStyle(); + + checkStyle(cellStyle); + +// StylesTable table = xlsToAppendWorkbook.getStylesSource(); +// List fills = table.getFills(); +// System.out.println("Having " + fills.size() + " fills"); +// for(XSSFCellFill fill : fills) { +// System.out.println("Fill: " + fill.getFillBackgroundColor() + "/" + fill.getFillForegroundColor()); +// } + + XSSFWorkbook targetWorkbook = new XSSFWorkbook(); + XSSFSheet newSheet = targetWorkbook.createSheet(sheet.getSheetName()); + XSSFRow destRow = newSheet.createRow(0); + XSSFCell newCell = destRow.createCell(0); + + //newCell.getCellStyle().cloneStyleFrom(cellStyle); + CellStyle newCellStyle = targetWorkbook.createCellStyle(); + newCellStyle.cloneStyleFrom(cellStyle); + newCell.setCellStyle(newCellStyle); + checkStyle(newCell.getCellStyle()); + newCell.setCellValue(oldCell.getStringCellValue()); + +// OutputStream os = new FileOutputStream("output.xlsm"); +// try { +// targetWorkbook.write(os); +// } finally { +// os.close(); +// } + + XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(targetWorkbook); + XSSFCellStyle styleBack = wbBack.getSheetAt(0).getRow(0).getCell(0).getCellStyle(); + checkStyle(styleBack); + } + + private void checkStyle(XSSFCellStyle cellStyle) { + assertNotNull(cellStyle); + assertEquals(0, cellStyle.getFillForegroundColor()); + assertNotNull(cellStyle.getFillForegroundXSSFColor()); + XSSFColor fgColor = cellStyle.getFillForegroundColorColor(); + assertNotNull(fgColor); + assertEquals("FF00FFFF", fgColor.getARGBHex()); + + assertEquals(0, cellStyle.getFillBackgroundColor()); + assertNotNull(cellStyle.getFillBackgroundXSSFColor()); + XSSFColor bgColor = cellStyle.getFillBackgroundColorColor(); + assertNotNull(bgColor); + assertEquals("FF00FFFF", fgColor.getARGBHex()); + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java index 8d427be55..4ec812d30 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -17,6 +17,8 @@ package org.apache.poi.xssf.usermodel; +import java.io.IOException; + import junit.framework.TestCase; import org.apache.poi.hssf.usermodel.HSSFCellStyle; @@ -82,6 +84,9 @@ public class TestXSSFCellStyle extends TestCase { stylesTable.putCellStyleXf(cellStyleXf); stylesTable.putCellXf(cellXf); cellStyle = new XSSFCellStyle(1, 1, stylesTable, null); + + assertNotNull(stylesTable.getFillAt(1).getCTFill().getPatternFill()); + assertEquals(STPatternType.INT_DARK_GRAY, stylesTable.getFillAt(1).getCTFill().getPatternFill().getPatternType().intValue()); } public void testGetSetBorderBottom() { @@ -551,7 +556,7 @@ public class TestXSSFCellStyle extends TestCase { assertEquals(IndexedColors.AUTOMATIC.getIndex(), cellStyle.getFillBackgroundColor()); } - public void testDefaultStyles() { + public void testDefaultStyles() throws IOException { XSSFWorkbook wb1 = new XSSFWorkbook(); @@ -577,6 +582,7 @@ public class TestXSSFCellStyle extends TestCase { assertEquals(style2.getBorderLeft(), style1.getBorderLeft()); assertEquals(style2.getBorderRight(), style1.getBorderRight()); assertEquals(style2.getBorderTop(), style1.getBorderTop()); + wb2.close(); } @@ -618,7 +624,7 @@ public class TestXSSFCellStyle extends TestCase { public void testGetFillPattern() { - assertEquals(CellStyle.NO_FILL, cellStyle.getFillPattern()); + assertEquals(STPatternType.INT_DARK_GRAY-1, cellStyle.getFillPattern()); int num = stylesTable.getFills().size(); cellStyle.setFillPattern(CellStyle.SOLID_FOREGROUND); diff --git a/test-data/spreadsheet/56295.xlsx b/test-data/spreadsheet/56295.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..d929d99a18201462f2b8ee50ccbbf87fc9e5c17d GIT binary patch literal 4773 zcmaJ_1z1$u76yifp<$?tmy(njN=Z>^rMr8OZb<=AN~B9fap;up7#c=EIt1x1DQT%U z<9+wa^S$Rk-*?WO@0_*IUjNzaUu$iZM*vI;G+bOjy&f!eu(c~Hl~ z*#3tAUXqX<->_&Ok19$ji*k7Az*{-rXGxzfP4B#c&^+<=d4a2!mxU80@3-H3M>2Vu z%?PX>(Wc#{rptKG}or}dD9*t|P`q}}e|&sYcaGCIh(foiLc zk;sBc7JY4q#k7S_puM3n;L2j{C^YYA#%ks)LM!Ar@O4Svrc8*(k`acC@Zf@Wdi1Sh zb{Q~#1F3Sql)7drtx~-m*yGek3736LzjQ9tIi*IUJUprCiCArjONs>fYGBc&t8fjj zOw?sVtm1l))oAk|o0SBb$|D?H%lzlnb4a&EG0>3L|Bo12)>aCEUXb9A)j z@wB)5i>DYx+cpSc@XiIL%yEZ2j(NBcJp6v~#R+<+>cT+#?bxx(D?c7OeSGNGfw+Os z`;Bsk8v+pko7hWamNP@K40L7{H0;2$Sm6geGQs*aPlN&IGuNlqH& z%FGc!J5UOLF(8 z>Ov1ssM9Su=QPI?2%;*R0FS>$9QN}k>LBP^5*$og-nWz;_rO+U?cdvB&5<>vmJ({AP@`_6?ivEKgnb!916`^V4yh(84kz~n2hln7`qxUn&f=a^0vQ4$ z2wqdGOff#y?;0yxo|5&1$a5qB_T>oz4PBt<$9jz+w`#zJvmR03(SbQzZopimMb~s`?L=3baN2*O)uMhvJ*_Ek-moueHpb zI1u3D0^%1Z!w>3wO(`Exs5eQa)u`=T*b{TH(j^U5cJ-ReIA~ z-zg|;Mfdz-!r0xKb!+Fee7hmg6eM_fBIgpl9=#pG(fvWAES zn-Q$6>PRyPI>1J7c!nE)qn)>|?oG*XTHK;bq;S>c9jN}^SZpYqHh}`8Z(-z|ZjVOM ziDTV#kjwj~DPOEcjZKTCcAVYwyAw}t_v9>=BIwOdnmwlWmZ#Ph>E^@wL=17PK%L~T zssmC^Sw%*M#Od#O83a2q-PcoL)QO@a9w-puF8A{OGZ2mdx0q9SEftGvwV2a;g=0ri zyu7e-&j^_Q$Xp*89RVK4ijrS)_HE(uRA7$ky6SciE1~nTxIon#JCQ4DHP>HisiJ0N3mQKa7T4D46)G!X66W=Lf(*&#cUX?6^ z5QR{U`}+e6Iiob|Y+0%3p125@3Nds(D2U)U5ZUPV8K>?ila<}qxnF*t)j9*KJONBX zYbL-ULa_NrL0oIfJC*xIEkVi`zt`{4NjE4C41GAr8GFd~$AtV(og5&JfhUJfMJ>w) z1f(+TiMkb?&>G&PkSFwn5+VL!iB~^#?^BUoSrPyZO$YCHx<~O>bh}!axUUgj782IZIn7IrR6xxRodA{#2X_bO z!TqwjVp5}fV|v!i(0F^Am#bZQBjF;5FZwiGrfe~C`z9WWu2BA)!E-EFIrBjyy1>56 zc4HB<>@0DAzEM^Nr^+*xZ*?9lgJnbYL*VsNsg->-Scy%$f91Yy>m8KB4K}`OY(;vg zgZ?`Xh<_^F->M&>rsy~cA%yQ}kywcf2n+|mu`Nm%t}t7Z2NPR z)gQO>=(ZQ(^2YWv zg;5k9z(J+1i)fo^UgCmux$HKEp>S-%A#0AKX0=ZHnBY(o^^lKn@Cvk+-P`Ftj6!SA zUm({$rG<{~i~#_XV)8a|hKbx^f$LWHEiPI!VzupJd6)VEXV%R2&Cc{SfJxJSHI(j! z`n!10!N^GCw=gT6+Za2QWz9NrV&kMK=j^HVq3J-!)xT)9#sq2HxY`*!Ae8Lj9(I3?}$Znny%G)K2A=z~?pq&uID zu3;bB*Dl@dr8LluL=f49I(GTn)z9VJ8VwWdO%n&2j5s2=e6(asePKqIk925pUK=cS zs2K=bw~LDQd&h<9+=*>84{vSC_9Y1!wGuHqd-Fl=DMh90)v|TpAa7Ioqb?3QS0*7pXwPrc7VzH-+s(@k`eQ|J>FFy^LMV{As^t7`tWf|55JM(Mp8SDI zru;C&RJ%Mky{LW%3tv6E?)FZd^y5CR$rbL{0 z)0v_f$@kD^BZFa4kZwUz#(A{?^!J1GuoGB=LB9}Oo_!RTMCxwy%drU%qU%LqHj6|w z4dnp2d;%G;n$!K8f-2d{P4;D_$tPz@50jc3*>0{wMuqX0q^wd^-$qAByk=awba(c0 z_WcfEo;()E?DqqSHm=5_HG<@M5$mS5yYO?;X>y{=B4-dw*3qQ~Z)*ex6UQRItsR|Y zG)Y3+nw?He`vY;nz;yX;>DGYF1I1Z_Fm;1Z0+7B<>opUkeYllk0(C2pb2390Tb*le zz6=sHOFmP|RIXKD?R$?$m;m_d#K0vlIt&32qp#a0xr=StP}HW|{Gpr@M7^w&Pm-u4 z*k^9=M&+X%H`cuIGJv}vbobrD7tgnvH)j_GhN5Lkg?P1Z@wds?+)}sFAPo+c_A|oP z;MPvs!%UoXsT(Z<^=uDK1p6;%R+K!TOuJq)isI-IOpcRzaYf^E3VOUYkByp}t-tO+m&A zJDwY9jk5U%>@i{D&od^zm~-Yn=SSlur5gE@Pnm+;gIU<3ZcKGJuQ4%Mhind$GRQI@ zpv27dAXV-*Ml~n4MYqiYyD1|}?p_Vywv~`X+B@q5<*bLtta()Sbd{Wcz{m95@A)3ygsVX=4hfF z)RNv5OyA5kgMq#cCen*|i^(rtKDDsmU_Tzy%7I9H zBHGP5z5lL1$Z4$E?RML{9syR$hXvJC*-co4-*Hv%W`BWlUYng2|i{t zhK|5_>3LMd9P2tAENwa-A>3~Z^m8KZjWl&!Xfi~|;7gpH zxYmu%xVkg#c|F9MGU@%}>!U(pEbv8xk8&F6F!zQ%UUcCpxUlwy`z*?HP=9ew1JWz_gyIr!eIHGC9S-%nSr_3~aP-e}Fev=j$_B z)Rrgex#_cmh>{4h^HX_JfwO6m;VMP`oM|9EFLP$aN|R@SChXm6-NKU$Mp;=WE2Alk zR>+`^XK4ibCTmSDxiuoNYp~C!4dQNw6IP$e>~?2LK+AbH!Tl8wbmJ+35UWKPrKgc| zzcV;o5t6Q|`!ZDkR>gps(5&(TW9yG>ypGuqv6ohJ8)5sq&yP$J>hX!=?lHV0)R?T2 z7p1A;k?7c5-rsZ@(b>4v#=9qdJO52dv$%#Tn@NtzYS-k_UY)mm6-Nn7b@LsGu59Q-YDR8&lbkyh zVZHu%&m|tE#kfGcIc$CpzPZ0}e|h@d|NA(j}kmjR0dv>#XS7vUSJWZ!7gSamZsEvj!kZ~qX8 zpH`_SU8G!;aw^kPota7Rl3NwPBXs}cth{f15ZueosKF6m>5|Rqff|rRp2#$I9ASFl zVl7BrY%H2cK|vBv^XY^NcImT`s>hgw2o-~hxB`tEh}RGfn2&D-m=AJn9vw0|=n44% zu!oyIa=*(UW}$KAqxaULg?O1fYPKS_u#IvOdwRSP_npmIqkBFhsS$u;d@nWo1uv3u zZA?^{qhnB@{oGNw-cqA>6#f&}_7{FNyxwu5I_{sKLB8t$_us#cbG^1j^`}3&c?L-ar6PvsF7 Rl20@=JmgOT6~I5H{4c*e>(c-L literal 0 HcmV?d00001