From 55b31d213d647cc3b643bdb473b185b8844ba84d Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Thu, 29 Oct 2009 16:43:24 +0000 Subject: [PATCH] fixed logic for matching cells and comments in HSSFCell.getCellComment() git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@831025 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../apache/poi/hssf/usermodel/HSSFCell.java | 11 ++--- .../poi/hssf/usermodel/TestHSSFComment.java | 39 ++++++++++++++++++ test-data/spreadsheet/47924.xls | Bin 0 -> 18944 bytes 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 test-data/spreadsheet/47924.xls diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 209d8ae8d8..0891579ed8 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 47924 - fixed logic for matching cells and comments in HSSFCell.getCellComment() 47942 - added implementation of protection features to XLSX and DOCX files 48070 - preserve leading and trailing white spaces in XSSFRichTextString 48044 - added implementation for CountBlank function diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java index 999fa287dc..701b7ce746 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java @@ -1054,7 +1054,8 @@ public class HSSFCell implements Cell { protected static HSSFComment findCellComment(Sheet sheet, int row, int column) { // TODO - optimise this code by searching backwards, find NoteRecord first, quit if not found. Find one TXO by id HSSFComment comment = null; - ArrayList noteTxo = new ArrayList(); + Map noteTxo = + new HashMap(); int i = 0; for (Iterator it = sheet.getRecords().iterator(); it.hasNext();) { RecordBase rec = it.next(); @@ -1062,7 +1063,7 @@ public class HSSFCell implements Cell { NoteRecord note = (NoteRecord) rec; if (note.getRow() == row && note.getColumn() == column) { if(i < noteTxo.size()) { - TextObjectRecord txo = noteTxo.get(i); + TextObjectRecord txo = noteTxo.get(note.getShapeId()); comment = new HSSFComment(note, txo); comment.setRow(note.getRow()); comment.setColumn((short) note.getColumn()); @@ -1081,12 +1082,12 @@ public class HSSFCell implements Cell { if (sub instanceof CommonObjectDataSubRecord) { CommonObjectDataSubRecord cmo = (CommonObjectDataSubRecord) sub; if (cmo.getObjectType() == CommonObjectDataSubRecord.OBJECT_TYPE_COMMENT) { - //find the next TextObjectRecord which holds the comment's text - //the order of TXO matches the order of NoteRecords: i-th TXO record corresponds to the i-th NoteRecord + //map ObjectId and corresponding TextObjectRecord, + //it will be used to match NoteRecord and TextObjectRecord while (it.hasNext()) { rec = it.next(); if (rec instanceof TextObjectRecord) { - noteTxo.add((TextObjectRecord) rec); + noteTxo.put(cmo.getObjectId(), (TextObjectRecord) rec); break; } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java index 7014cb5149..bb4707b321 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFComment.java @@ -23,6 +23,10 @@ import java.io.IOException; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.usermodel.Cell; +import org.apache.poi.ss.usermodel.Comment; +import org.apache.poi.ss.util.CellReference; /** * Tests TestHSSFCellComment. @@ -215,4 +219,39 @@ public final class TestHSSFComment extends TestCase { // wb.write(fout); // fout.close(); } + + /** + * HSSFCell#findCellComment should NOT rely on the order of records + * when matching cells and their cell comments. The correct algorithm is to map + */ + public static void test47924() { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("47924.xls"); + HSSFSheet sheet = wb.getSheetAt(0); + HSSFCell cell; + HSSFComment comment; + + cell = sheet.getRow(0).getCell(0); + comment = cell.getCellComment(); + assertEquals("a1", comment.getString().getString()); + + cell = sheet.getRow(1).getCell(0); + comment = cell.getCellComment(); + assertEquals("a2", comment.getString().getString()); + + cell = sheet.getRow(2).getCell(0); + comment = cell.getCellComment(); + assertEquals("a3", comment.getString().getString()); + + cell = sheet.getRow(2).getCell(2); + comment = cell.getCellComment(); + assertEquals("c3", comment.getString().getString()); + + cell = sheet.getRow(4).getCell(1); + comment = cell.getCellComment(); + assertEquals("b5", comment.getString().getString()); + + cell = sheet.getRow(5).getCell(2); + comment = cell.getCellComment(); + assertEquals("c6", comment.getString().getString()); + } } diff --git a/test-data/spreadsheet/47924.xls b/test-data/spreadsheet/47924.xls new file mode 100644 index 0000000000000000000000000000000000000000..287f17b6e85a037fe4564bb9fe48a74c7381d970 GIT binary patch literal 18944 zcmeHP4RBml6~6E7W_R<`Hc8V^N@=(0Uz(;N&8DHXg-u(qAZZ(+4poaKNjGh5NNU(( zQ7WmxjAM~04VJM)1(g}pinXnXBif*jj!sZUMMS|VN=2ljX~j_-VEw*(_wBxS_q~^0 zrH*xYnfrduyZ1Zio^$TGKksgy{AtCB2fjA#d7&Mbihy_{RV+d_x`BHpd6Wwg#SNd| zNTpJeh7)ezI?f>tybf7s!)^hjAW{($Z5~F7AQdB(AeADOAx%J55 zRUu75szy2wX)4nBNYjumK$?zpA<_(_nMm{-rW)J*Ii$M(3sJ8Y{rF3wEnhDBa2*gg z!y5S?)$kZg5hjX7BRmT+BG2OOybKPXF|`zIF}=? z@C~ew^JIDcs?7VCJYRx0yTp4$`8%$dwG=tnPi-_?Wxh|ybB#Pt#MuzvkY#=?DT@)E zo<;tM*hdYSVp`Z(4NvHWCv?I?dc=P#$8>Q&1bC;CLm1OS#saTV7-CkOs5uA?LjefdjPR;E4*|QrrFIc{*)_T~qU`B8j zMgr)wEfU_p_v*x!>ztWtFjPRNZe?m{bmHKdQ1c;bjh;(GOJ}`U)QKh`HdBW88RN{o87ODS?9zNqi zSMucMSMuc2zuf763rs61bnIGiCl^`rcRvnENur=sHC(tArq z@PN$EnUtb;cORd5e5MF~L()TtA(H+NWRQpDp9?(b^&a#(S2|~J5?_laoY6^o#=gp* zIm?suUx;?xVzQ>>;mprQ2dtuVHkhVIaf?TduJPr}QRdI+NBKWzlWCmmaf^wg!r=^4 z(lc^?Ijy%eeYm2cy0jX4J}Bv&jav9h&r1Ixj8i0C({q)TUz3xw*|ax)Au`9MV(N=8y}CQHNbGjdnZd+Nx=PL(ILu@xiSDItB6?&qw}2Xn7EnM9 z)(R-V=}SQ(aSl;X0WK^QRDgY8K?TBv7NAiI;=pEIPyx30f(o#q7F2*OxS#@T+65J0 zn=hyUgMn{>m0EjXGzfUNhoEdIiEg@_8tFx&yCXUXLq{GUbVF|t#+f`o=$hUj3_f{) z&|SSj7?JV-p$mJ1Fnr|!Lbvt?VXPY$WVFrSI$RFS1H|7tQ}YAyw+h0;Ot9O+i8mVTZfCXSxG!W{H?=P-8?}2t;1#EJV5-d6U`6A z-#WAM1M#;Gro7`C#kGr@76l61;`PWofq@|bGL8@IdkSr1|*yYzV+5y zQ_=JhaiJC4Xuy_7i3@FaL>HzhheZD%k2SUNK~iylkXSw-dUvDSwHY8;N64v8t~)P5fovO#$8U01 zVrukLIqv-CwK3f&Eg4m~Hhil=rv{CTP`U1Lu>)i{;o%;smxpa2dTU>rQ4e*QGoQ44 zER`C~fLI~u$dMyCt=&$uT03dLmWS2a?X+B*Ekj_PP2rvhZ{>M zJ1sW^VX4>Vc{!TOEx@mY-mYrwj=W3j4pyK*(0eXJz3WZY-h8d@AXk!0GLq2UU<{`| zhs;WR6~pANsc~*}QFLWjSE4VuczwJZxnMyR4hVTiEocxLVbBu zVaC9>zHl(n{hO1gQ$@U|n!s)@*CxoOLTA&I%cjPMjS57PO;Muz%yZAWv#HeC#B$lp z^I@Yxm1Gl2bpPq}bM9;=>uj2H+0^;4VK57z(Lqt+MEA)Pr`*|8>1>wdvT5*P!w{He z6M=31{8x82Q*<^hxonzz*r=c;Yg3%)e*L9q+}TuLa-`>?Tl+GyL;FFfYX=6s!vi>EF0VPp5S(YDc% zU%RuJrn7PJw2OS$*gb8u?UCpH=+5Q>osEm9#eCSTFy*tHYxZGd_q3+i zKS%bt)us%er!jBM^qVWSe)Gaq*t4$`V*Q?PyMxZog2JP8&{-O&-HY()qkXMA=$tGl z{6`0!tAW~mMu_{q^=^02xmi$niw?R#1GReyHh69MoIB{eEGT?K2VJCr+WkR@s}B9Z z9dv#c6rP}iF4jQp))(UM2j1=m8WFV|maXd8vOluSX?f#F_42odn2`^;@CTskxD&TQmH3PD)lsE2F1d`Kx%&^>J+6Y z04A&DPABel;=#Q2gCXRt)$)e0OF5f2D&C8p;JQqiMT0? zz3Y)&8V18C(V&$Gr^<3lIC)JG7jZ}I<-2zDcJy^5`v-20Zb#(d(W2fD#7ghS>;OfqcXhfn_w;3^J!fUBW_DZ!iV z;E07L$hdWMpz!z!sY(wK(hGjG^ zc>IUWzZ?T3#HF^LzU_%E>Gpq>Cd^dCr8a5k?J)WlM+69DyfG7ttmwl9jt~@ACN75j zVqIcqa-aj@MGMTvbOh#g0A@`G;8P4NYDOdDDONbF`VM0mB<=8UsX>9}7<0;M2(VlS zPalw*Aqkb%zJc59WVIN({Q$A9KbhDc>2zY4C1@|iNJ1zspL5nDMZr3W71FrIkqxx! z$qH%e$qH%eX~--U%LG=@*Y8Y9i+@mKIt9?{r4Dj!ZV&m4kPe%Zkd;I|S(2zHOA__O zQ1-ULBIs_8Q-q=-7@{3@9>`4msWQ6ia=Pkw;P05 zAtCyc^Z;$`A#lEgpu&~)gVL!S>kHJo6-QKUttYxzg}4MZ64!PN^f6?q@EUgtYL+5W zUkeTcXjfL$ato{M#HEJsifj~j*%nFmp>`Gc<96FlS=(zM^3C#YE$Yr-$Svc`pv4|> zlk}i$4z{4J=tYwVl`Z0WxwZHX)as{PYldfBvn`QGE+*xjT(`3*O{6@{MKg;MI~bWN z4%*~Wu1(Dg@p)|9{=DrS%iRfAH}Lnk5kbOFEnYUn%h&{s@9|?-GlK|?@e?XuOALOd zAtn>=W1@7d64&F{s~Hh{jFs30Jz})t+$v_A=0|!=#Ujvu1YdI_f5#s3*Tg*fx=fI& zV=+%nJjHP8T!K?q3s12W1|Jb|C3JxDqAOC3gq!qHoxYml0P1zX_GZ;dL)-`2eKs29 zy%V%MZ8QRKvxVBuUhde=UTRkHwLB>8Ni5=j9qaNFpar&h6BeB$?<=HU$|31%Up)P@ zwVmzp&5~Xx=?mrgqmnX+Jk~K6Elr4eV-Mau9we_RNVmz{x6AW9@4W;iPbN;r=d?D6FBE=Yll=s0;$2MoP1 zqDiarzMe{jD@>9!KIk--i8t9JJS^UDOYTgy0j=C&@FPh)eCOcP8#8F@(sR z^yf)HQ%_ia#1nZwjkfYAuI;!^#1`d=;*>Z+l(AnSGwe4;wa};ea|R%cjew4_W3`}R zv-+5-Hy&(N9{eEV zd2yRO-(kE0fsPyHkm`H#UIqjaFiDS_0o3kzfVIY>Y<=t!$B+i`W(;!Bh!tZvVNlx) z&#J^v9}H2^O5P``ING2mSBr~3)O&2*>DyZmzp(YD_D`)q39F8$q-RZ#7>^j0qLR$y z!5>oaSV0|k#8${h_QL#KggQ=%eL0Gd{_ar3lV?Q{CKM2WB7(SvT@_(M5#-%EC98CNC#?N$$@)(pcxmeBQ0r$3Rzz2erHE4L5d&1Bt0ICqig@U( zD1uFc4Nob=t!V_I2=W%yS&!(uJNku_FRr=#uF%8N4u1c@u7VZO(PSw?H4VUfD55Aw z5#r3*@Cc=dX;KjZ@dTvNV?+@YLEhKf6frWV<@>+deS7QuA1l58Ft``OzBFsf&!Zo0doRXRdzCvVqImznh3q|xB@BP#g*$UAhJRaa*)WLWQhMMrzY&<# z7b)N_jC0SO(HARdjk>IB+1lZMbM%!~EiTdu^075bbg_bo-&d(o5V9#360pF3Y=xKb zHT>MR0+wF?r>$T!>;LMl3LXp+>i75@_%bpup0Sbh_xYSy@%q;B|EGaBunbb9<|MFk z_Ml&!xZ|^DUR}Sf{L6Po#KQSUpJ@c`FcN)(?^}`B&0m2;U%4KM3kVosvCF}LnE{61 zN7Vm^cnDW^NWVuR?iOaJVE<7$9>=Ax?dclmzp=kN8NGa0SE5(SAVv?B@c#?6qWs`% zGxr|FJ^eYK!>|74Q%^iK+;Bnp(7mXC{i_eN{wI*Iur2G~jw^rcun~zH+vjv$WTe}XxWsWM67_Kp67}D4^N8KpP=c_Z6%p3+HLjVDSEkpm`5A%>KYp?a)-923i zWwvNjV`D6uVJ{=js-EudrI=^%6ACQbAZyaFRJ^k6$Okg)-nw~{HDIK_o$bJ?bfnT-zs literal 0 HcmV?d00001 -- 2.39.5