From 263c8a28f4e25a36243833e57e82b2481e250423 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Fri, 18 Mar 2011 15:18:45 +0000 Subject: [PATCH] Fix bug #50846 - XSSFCellBorder needs a theme table too, but as it gets created early switch it to the same model as XSSFFont for getting it later git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1082946 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/xssf/model/StylesTable.java | 11 +++++++-- .../usermodel/extensions/XSSFCellBorder.java | 21 ++++++++++++++--- .../poi/xssf/usermodel/TestXSSFBugs.java | 22 ++++++++++++++---- .../poi/xssf/usermodel/TestXSSFCellStyle.java | 4 ++-- .../usermodel/extensions/TestXSSFBorder.java | 2 +- .../spreadsheet/50846-border_colours.xlsx | Bin 0 -> 12381 bytes 7 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 test-data/spreadsheet/50846-border_colours.xlsx diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 5410a7fab..e2e772a3c 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 50846 - More XSSFColor theme improvements, this time for Cell Borders 50939 - ChartEndObjectRecord is supposed to have 6 bytes at the end, but handle it not HMEF - New component which supports TNEF (Transport Neutral Encoding Format), aka winmail.dat 50313 - support for getting HWPFDocument fields diff --git a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java index 60dad91b0..e643f9fa1 100644 --- a/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java +++ b/src/ooxml/java/org/apache/poi/xssf/model/StylesTable.java @@ -98,9 +98,15 @@ public class StylesTable extends POIXMLDocumentPart { public void setTheme(ThemesTable theme) { this.theme = theme; + + // Pass the themes table along to things which need to + // know about it, but have already been created by now for(XSSFFont font : fonts) { font.setThemesTable(theme); } + for(XSSFCellBorder border : borders) { + border.setThemesTable(theme); + } } /** @@ -144,7 +150,7 @@ public class StylesTable extends POIXMLDocumentPart { CTBorders ctborders = styleSheet.getBorders(); if(ctborders != null) { for (CTBorder border : ctborders.getBorderArray()) { - borders.add(new XSSFCellBorder(border, theme)); + borders.add(new XSSFCellBorder(border)); } } @@ -251,6 +257,7 @@ public class StylesTable extends POIXMLDocumentPart { return idx; } borders.add(border); + border.setThemesTable(theme); return borders.size() - 1; } @@ -437,7 +444,7 @@ public class StylesTable extends POIXMLDocumentPart { fills.add(new XSSFCellFill(ctFill[1])); CTBorder ctBorder = createDefaultBorder(); - borders.add(new XSSFCellBorder(ctBorder, theme)); + borders.add(new XSSFCellBorder(ctBorder)); CTXf styleXf = createDefaultXf(); styleXfs.add(styleXf); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java index adc695b09..b100bc210 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/extensions/XSSFCellBorder.java @@ -38,19 +38,34 @@ public class XSSFCellBorder { * Creates a Cell Border from the supplied XML definition */ public XSSFCellBorder(CTBorder border, ThemesTable theme) { - this.border = border; + this(border); this._theme = theme; } + /** + * Creates a Cell Border from the supplied XML definition + */ + public XSSFCellBorder(CTBorder border) { + this.border = border; + } + /** * Creates a new, empty Cell Border. * You need to attach this to the Styles Table */ - public XSSFCellBorder(ThemesTable theme) { + public XSSFCellBorder() { border = CTBorder.Factory.newInstance(); - this._theme = theme; } + /** + * Records the Themes Table that is associated with + * the current font, used when looking up theme + * based colours and properties. + */ + public void setThemesTable(ThemesTable themes) { + this._theme = themes; + } + /** * The enumeration value indicating the side being used for a cell border. */ 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 eaeb01faa..710ba2c01 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -742,12 +742,26 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { * should still be able to get colours */ public void test50846() throws Exception { - // TODO Get file and test - //Workbook wb = XSSFTestDataSamples.openSampleWorkbook("50846.xlsx"); + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("50846-border_colours.xlsx"); - // Check the style that is theme based + XSSFSheet sheet = wb.getSheetAt(0); + XSSFRow row = sheet.getRow(0); - // Check the one that isn't + // Border from a theme, brown + XSSFCell cellT = row.getCell(0); + XSSFCellStyle styleT = cellT.getCellStyle(); + XSSFColor colorT = styleT.getBottomBorderXSSFColor(); + + assertEquals(5, colorT.getTheme()); + assertEquals("FFC0504D", colorT.getARGBHex()); + + // Border from a style direct, red + XSSFCell cellS = row.getCell(1); + XSSFCellStyle styleS = cellS.getCellStyle(); + XSSFColor colorS = styleS.getBottomBorderXSSFColor(); + + assertEquals(0, colorS.getTheme()); + assertEquals("FFFF0000", colorS.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 83edc8f2f..02b34ae85 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -59,11 +59,11 @@ public class TestXSSFCellStyle extends TestCase { ctStylesheet = stylesTable.getCTStylesheet(); ctBorderA = CTBorder.Factory.newInstance(); - XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA, null); + XSSFCellBorder borderA = new XSSFCellBorder(ctBorderA); long borderId = stylesTable.putBorder(borderA); assertEquals(1, borderId); - XSSFCellBorder borderB = new XSSFCellBorder(null); + XSSFCellBorder borderB = new XSSFCellBorder(); assertEquals(1, stylesTable.putBorder(borderB)); ctFill = CTFill.Factory.newInstance(); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java index 46af27d59..e7233c227 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFBorder.java @@ -40,7 +40,7 @@ public class TestXSSFBorder extends TestCase { right.setStyle(STBorderStyle.NONE); bottom.setStyle(STBorderStyle.THIN); - XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border, null); + XSSFCellBorder cellBorderStyle = new XSSFCellBorder(border); assertEquals("DASH_DOT", cellBorderStyle.getBorderStyle(BorderSide.TOP).toString()); assertEquals("NONE", cellBorderStyle.getBorderStyle(BorderSide.RIGHT).toString()); diff --git a/test-data/spreadsheet/50846-border_colours.xlsx b/test-data/spreadsheet/50846-border_colours.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..49ec5320fdeae06ebc02d104ad65047bdbf9a56c GIT binary patch literal 12381 zcmeHtWmKG7)+X-m?oMF=LeSvBT@&0PxVyW%hv4oSJV5YZA$YJr;qJ~(8u+X-Xx+_<)~>$uQnW+ulVCw< zDaH#&?na4mRMulb0LH=z64paUf`UTH7YPm`cge(Fn+#LhH7iX~LO8Qb*|HoKhWc-+14=YmN zp+uX=`t6Rj1YAx*tBgb|xeCcgssqx7s5~mc%Zorre<{O-iHps#mv5F1Hq2E>i40ie zdRpj&YS;}tW-MGQ-m9Tz8&H^b-y79}qx?(*+ ze-S&MCZ8TThda^2bE@2MYUtp(!~Fa&dR6pxBvzSUna|P<@d>t_+@_I-_+*=&-A(x# z6w*g0`P-I1CzA_^8GU@)+F~v3X$LfoNv_fuu)!TQLgNnK87E^*eNELrNE7yntg2_7 zD|j$Pua>aKNSbwlQ-5H~cZxS8Rk)?xVp_-F?KO6A2y?JE0#ej@{QX+`1(7^F%Yh+% zzsr0`=Aywd;_tjA!)t>_ClD*5K;9A+L=6yw{>+_7C23H2!}_MaLku`1*Mk!c^;Kz5 zol^nOl`mceX}oYYB7_*eAZV9}0q5G0Mx#%9UBcKIW3u#lU~OAK-3WGCGS6_{%7qU6 z9Ul2PPa#W0znW;+sTN{oWN<3U$BFIr3llAZRyR%)kyydjRFfs#JThBJ7;YCUW#2wo z3(HadMrYy=-4M9UkVnl38b$1I_RO68?2&suFJ#h(pblhfM}%(K!=QWlhg{MCMDHS% zOZ42FMO_v7PKlfjv8ls(AIb7z(I*8O3j;|ZTbT|Tl~fn1*|WRy*|?jaH2G(nlX$a; zSm~vp@;%G$AdqzrRx{7>%W70#NW&N07M8rO)s)hA`vW{eb;8gg%n?=~I4 z7B5fZ_tP>ZQRENIsb2~29K|~P$}-Wgy4HVudgko@D(BoaxUssCGA!s_n6=mmp|3W0 zwKdoti{K%GuM_0f)H9ZOj7p5i8g~nN08wu9&;XEDwgaU9zsIxrUBbs~P<9zi_#?yo zxk8R+##Y8GKR?-iSoogim^GmoUMJ--N?v-We@Z23M$_UErgg`HJablgv?w&egbx>%^c7J6Q+wo{k*F^J!h0f_EV(Qhlq`PVL;2NHN)g`K z5;FwuWG0$%W)oyoQPeA)m~ZMN1{w^m6GuZEV~iC#Osh$Wp(U7R4vxg9DPIbp0^UJW zDns3uj7t^0?yMC6+JMQ7tj+H`muaV+u!HU0>NQVMkJZg+rgsr^lqae1e@XA5MDnrPEJUBj3mcaK~54ge}TL7MXY9E1@asAnn`UoUPkG-igrDG1wOuffas$b=>C1 zJw`cmEywstQ7Lf$%Zty3%SpVkxp-D~s!+@8)}6UOZft!PIgNcbt!gN4Ey2zuJ700Yij2`yo+w4L6 zs2F9k0@cG2?%cdQGIOa+bgY|GGUK@>wrctj>Pm%r7iDl|W(A{w3Ii4Pim|xoocIPj zHUt2Qp){qfnzzSgNSFMh^jTY|MOs`ez{c zx|~qt8TS!Z6)=B_b73adIhhRDt16F{B6jGMn`nFlkZOsJNurE6?Hx3zpE=rT2ucs} z%;abzCvsSQn<)6TfCP#SafN44y}R(fq}Q_46KBNTB=rpx_J_f0@AvGp+mKn+@7ZcqXAbTZ`R}3KUi(#$cf@ z+E$0$SW-2z1?m>6&x;9FJDCua;t_Hi%=r z<60WvHdF%Jh`nPS$=gX^xXv<==t=N6CNpshuSE(#afI^xaEFbz?QZ3&RU9jVDQ8=pxwo(TS-oINzVq?38|>@}Zv7)Gf6(SUG#X5F7*Rf)X1+xG|e4ZI3rdlX|fg zAf5c|8Ge$oI*r=Cz#MgXk;h8>(gHu0aA}5hd?R=OZUVrVWFN7zSha-f!x}W>XE!42d08Q9mCu@Dgut%Gx=A$`H zkA<0(QF33{umR*?bD^01Vjo~{NwEd$UYcwnH}p5(`;~VX!J_69=4XAZ)XfV@v+JhNIGxEiP*{q_}>LI|C@z9 zX;8{R07^OVex)1^X2!;j4lF-!&wfO;NJUxsZb7v6j4RUT4k3+V>7GM*}nxUg3$X^C{Tq~pNCH`eoH~j zPXNsB|3m{niTF_6gBhr^)5h<^~bDOA5 z30vyp4BGVBrrMpCD~p#J$rzt28K%ixtS-o#Im^~@0jnUty;CNJK?JhaXUolKrMz#Q ze9`B7NPk!ULj{A>@p|0368rhET?BG9eL_a70Uq|$R$q}vaPGGKroRA2Jq5mcl{B%K)%Ca^K ztVr#svp#5!y6Z-=3DAuZ)|xdWFi67!qv10B!p6!8srkEAEY!3@D7yAni-~nBID6LWtcgIK8^kP)}9P_JSNq2~fOH($>iLa34Yaq$G;6%!e zUahN3iA@SstwHf45t*}{rC;GWGu(i+42rcY*X4dtZEp-9LRVY$VXd~R@Z(JPLWPVX z1J9&6ce^NTwV6P59 znKr|FRoXL(qWHqBxiA%w50^(XM{_neyl)fdx^!ByCJSL*n%b6sPteJnyk^|@HQhH{zK25d@quymjW=WzjfoE*T}>2`1&E zqA%h~u+qw}8aX4Ar(q@03+(t}sDKEYngvO9W))mR=$RyoXh*9HeAIgbni9`kQK&lYPx3j2lf0C-KKOP-OnZP~uICxB; zr>c*QL$q`Av|ZPferG7or0)mgG1Xw}3jO*FI>bxhZs=fsoLSP;6kJgv)imfiwV4mq z12)=n>xoSD5IA-4Ow=RKVt4EKs5!j&pm_y0ww`!@=-QF}XK zXFg=U2tD+;@|_CQWMfdpbn@C#Sj>JwT_{+FeG~!=9JRur5&Xu+6Grke=Ri^U1Id3n zNVXq?+)I9Ii=~RwZ{vy_C#=(zgl+JAK$>wbkT2AT`t0m8E2hbp5l2*gAlp=0bDH_q zwsotAFvVNFu=FFP*=|_$P*k-w3T%Y+W>MLxmg-7r2(zzvNDy{FbP_~W910`(ODcV^ zQ{($;&s4cmn`MvD_RF)whp}y;&ZF?%5SaShX*Q$HXUat3uciW?!QLnm5%_PMZ9`Ug zzX}yh>$H8pwQZ+O+&-mF3Xb~DR_u0b4Cj>}E&e{IcGRQyr}16d%O&%egH3Ul74F(s z%M0R^hBwiVyV$V1J3ckLI#g(0%pS6EY|PJfV;ty8Ip@%$;$t!GB*gIO=|~RP2-SjKM>%Dxj+=C?wxg%NG zIT9e+S1M*%MN^H!gi1!$SHg(67G%LRpErgdhG6cMBDXhG#potr38Uj>8eusd=qn+J z6sO3;B8ea{oi!gCX@Q4D^biYP#L%#Hmm~O$t`cb`*N4#aG73E|Olx+`!7>er7*(2F z2?NSjJaHtrTtG5WIG)sH#L%upyx7F=lxc^cX3{}Rj*vw!1|ibF*2ey|yPC6wxr879 zv9b;4yHQeCdS3)Mo_&{riUy`DI;4Fvk`&mM>D*4lMK?9U7Sm_NdIcy#hHQlLP&HS@ z4D1ak9mb3t34Q#Dy2$AaaNo>lV|w7XNVd=zfnsn3YNCQGfyFfj{h6Jc#!T^JYkGXq z2cbFgjPd2~Wi^iV5sU120<&Wa@{4XhP|Sk4N`*hn4~Sta+5Ny*&IB8LQb5tYnb4ivg?5veMkYD^wjR1XB|kHfaC8)iII#0s`d(Cc3#P8zC1tC&T#FTP##A)!92sEiiA0QC2R0dF z4T5^|(#WTA4C-`hrU5W1&hK=I=Wu}&Skb6ZoRmFga)=ii2xKAvPrI6$Iuenli$xL< z1Xcs4bA3%*ia^-!u>0d`Xz?T)R%#(|Er1O&yVUIZE9V96Q91J()w9cIe1xgrnt4>f z6cC80)F`F`$)N;5 za0d3y!87Ge%C+szdGE1-oBO4WnOietiV;xdFx2!)+!To|>Sw~!*Fq-bBEhzfE~$fL z7fcMvSe@NHt%1fbsXi_01kol-jLGp`sNf(^zm&U1|6V9eizfx+ z`Np(z^`SE*d{dA@*_V6|#w)Fu$B%9G&$vZ7-s54XyytjtzI-!SDt|J@ zY3=TlYo!eQ{?4s%rMee6jP$h+CdwHqN?MW`khp))DU8c`j&@ppxp~o>N`>EWCsrt< zf3Icf%FN43+EZkQSKhmU!)=Cc3}-SYvBHnO5NjtS+PvZ0mK%POG3@TL&f(3g>(%L# ziG?iQy0w{Tfsvlqp>JC+6Ix-bqHbGwc6S+1m12-PdD@ICufMew9PhJ`u*hpJB_8{^ zb#1IgNi(AGIo*iZ3r%k~mOKthmk#r5PiszGt8LWiTMRB|RweB*;=4N9EGTx=c^L*@ z)^3*DI4PqDJSy3CG@QDOFw(ZWJc=MT5r5feGEGes7SR88^^$b1rkhf8G%}`>?L*x` z-th23Wn1;L&13$gm`FMAl~ynNUEsZZHdk?JbK|R0)JR*ewwq>#gSBOWTjcXkv517l zZKmSd=BJY81|M6!?&YJevD+J*4rTY4 zbrnD)p5OoO^$6?t2uguuqV?{^h6xgR=4~TE82SR zG3i`Jj6{0KPrJ;28Sm&7>vRO!rxrDY7G{9u`#q{yB`v4P0Fzl!k677%D#{;;xGUGm z+atkV*3EFGmxCa(>?2H8J^;ug;<3rmJA1+}VM2MRp!1DW{Mn|ovcbp|E+hEZ_bU`> zTN&1VTUexX2~pbo6-p(aj22k9=o}dMAx#+y|Hn3z$a;0sv!PG)j4ofjiJG}s^1be@YLApt(|6w!726?X)|phk-pOv#N|R};`zROe(2%^D zsxzp=H54>7KUBex52iqVH&2G=bcfb>ePy+e5)yc{ zB(G!f-}O7%y(G^97daS~kNv%Mg!XS^z6TLc0hHjQ{h4_BwzmJ}-4Dk7ab+fSS%b<( z!H19+D8UC&ne}+Vf@*B}jPA45V3Hm)Mm*>=(*Bbi)XZScGm2jrom zO$ZJRbQkzrq4#8-<~J#nagfi&DP3zY=D+=BtE(#B6EzdV`C`3}&!=BMWJQXOrF$aw zJo*$y_w8%<#gl@Na{;t!EK=cWamIXRplM#7d`I`*np^<3CjcS$jh}ZU%;hs&Pfu*d z3IJe0Lv4r#?Jg0`C3`-HDJe`c9A*>Q?2Ju%gEz{mueORoC_=efTr7_WL+#D9Zp&uxSxoHmY z+?O2sw(*YPcpX_i6!+u$_nj~)$2-SyI#7o8C0~fYkGxGj=$T)p3)Cmvl&qOAg`;M& z9hZl?d+^^URLW;5rJ%$_G>-LZw1eI4qh7xeYNcCFwLOrQeK?^|jA zxrqO~|E954LHe%%f9?GHr{Ir!Jjhc1)Cc%f@ULxDe-`uxHD3JtrmCknPn&6eA$;%l=6>Z{R!~b$;(r~rzaP`0GmP8!hZ|+>m1`L;J;T~8}?Jcp9B2~^%Ui4 zG5goB_;G)W^0T!46y<5{^B0N={!f&@RYIR4JS{Q)LSP~I1L0o<$ETuC)2?5l2}D0d zpVW