From 0245f6ba224cbb9fb5dc80c653b2d9c6da1a2ab2 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 9 May 2025 20:58:06 -0700 Subject: [PATCH] Removing Columns/Rows Containing Merged Cells Fix #282, which went stale over 7 years ago, and is now reopened. This is certainly related to issue #4379. If merged cells are in the midst of deleted rows/columns, the merge may continue to exist after the deletion, leading to various problems including spreadsheet corruption. The problem is not with either Reader or Writer. This PR will automatically unmerge all merged cells whose first cell is in the delete range before performing the deletion. This will address the corruption problem. Something more sophisticated may be required, so I won't merge this PR for a while to give me a chance to think about it some more. --- src/PhpSpreadsheet/Worksheet/Worksheet.php | 28 +++++++++++ .../Worksheet/MergeCellsDeletedTest.php | 45 ++++++++++++++++++ tests/data/Reader/XLSX/issue.282.xlsx | Bin 0 -> 11145 bytes 3 files changed, 73 insertions(+) create mode 100644 tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php create mode 100644 tests/data/Reader/XLSX/issue.282.xlsx diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index e9e0d78c6..3703f0003 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -2392,6 +2392,20 @@ class Worksheet if ($row < 1) { throw new Exception('Rows to be deleted should at least start from row 1.'); } + $startRow = $row; + $endRow = $startRow + $numberOfRows - 1; + $removeKeys = []; + foreach ($this->mergeCells as $key => $value) { + if (preg_match('/^[a-z]{1,3}(\d+)/i', $key, $matches) === 1) { + $startMergeInt = (int) $matches[1]; + if ($startMergeInt >= $startRow && $startMergeInt <= $endRow) { + $removeKeys[] = $key; + } + } + } + foreach ($removeKeys as $key) { + unset($this->mergeCells[$key]); + } $holdRowDimensions = $this->removeRowDimensions($row, $numberOfRows); $highestRow = $this->getHighestDataRow(); @@ -2448,6 +2462,20 @@ class Worksheet if (is_numeric($column)) { throw new Exception('Column references should not be numeric.'); } + $startColumnInt = Coordinate::columnIndexFromString($column); + $endColumnInt = $startColumnInt + $numberOfColumns - 1; + $removeKeys = []; + foreach ($this->mergeCells as $key => $value) { + if (preg_match('/^[a-z]{1,3}/i', $key, $matches) === 1) { + $startMergeInt = Coordinate::columnIndexFromString($matches[0]); + if ($startMergeInt >= $startColumnInt && $startMergeInt <= $endColumnInt) { + $removeKeys[] = $key; + } + } + } + foreach ($removeKeys as $key) { + unset($this->mergeCells[$key]); + } $highestColumn = $this->getHighestDataColumn(); $highestColumnIndex = Coordinate::columnIndexFromString($highestColumn); diff --git a/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php b/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php new file mode 100644 index 000000000..731c03671 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/MergeCellsDeletedTest.php @@ -0,0 +1,45 @@ +load($infile); + $sheet = $spreadsheet->getSheetByName('Sheet1'); + + $mergeCells = $sheet->getMergeCells(); + self::assertSame(['B1:F1', 'G1:I1'], array_values($mergeCells)); + + // Want to delete column B,C,D,E,F + $sheet->removeColumnByIndex(2, 5); + $mergeCells2 = $sheet->getMergeCells(); + self::assertSame(['B1:D1'], array_values($mergeCells2)); + $spreadsheet->disconnectWorksheets(); + } + + public function testDeletedRows(): void + { + $infile = 'tests/data/Reader/XLSX/issue.282.xlsx'; + $reader = new XlsxReader(); + $spreadsheet = $reader->load($infile); + $sheet = $spreadsheet->getSheetByName('Sheet2'); + + $mergeCells = $sheet->getMergeCells(); + self::assertSame(['A2:A6', 'A7:A9'], array_values($mergeCells)); + + // Want to delete rows 2 to 4 + $sheet->removeRow(2, 3); + $mergeCells2 = $sheet->getMergeCells(); + self::assertSame(['A4:A6'], array_values($mergeCells2)); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Reader/XLSX/issue.282.xlsx b/tests/data/Reader/XLSX/issue.282.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..95751ba13e683774aef60230adbb6a4727c19559 GIT binary patch literal 11145 zcmeHt1y>yFvi9H(LvVL@5AFmB7Tk4U2yVgMJsCVmaF^f&ch@Ai1pUYiRu4i>uS3Om4RjDb#!Q%oD0muLVfC|8S))XBE0{}!L001}uWLN`f zM+Y~MgPVz_mlMdvIyQATl+;V- z-Dt?G68C{M=?=X`3PZ6ix0fn)%+z~j&0mzH6VqK)=y|Mtd5?@vLZj@C8x||&$|Tm{ zE$-{JFFmZcf|w(_Nu0VZD)Hra!L{vt9R@N5QnjKpWft)f`Wm0iur$LWja3$3dz)S> zYmnupcjKO~g6pd@z7UC~ikOxRU%?xAx*29pfP6}6b7{0lIpC10^d(~__`C8n#l23W z3VlH(v^;}q)-(r^=!H7m=nN(}ZQDjlHV{7DWr+=z9og_~;d|1L`&6NN4dj+#BU6oy zk~nV%jhWTdpCgA0$s8Ib&y^wVt+`dNPj(la*0!2a_1<$IbGYx{Ct{{_NN05haL9&V zZ*TOWMbv-)Hkq@=C+6!x?lrqhO9QI z;XOXW0o4BHy;<5SFeCfE5dOBW!DfM zsW#h%m$YOGmO3KU)PSWFoXS>BxeD~O>PwvhmH0*_vQvYWhs&8C5{K=+R(KYsq2Fr9 zm+Pbch;BIXV|hp5k#EcNLautcwpiPNX?Wr6&hCc{zN?KQ3l4;lU97<+*|ix){-rFY zMt_iRsT(QTwo9xiyr%6w%G;KX;HwpB6{KwQDg&Z#H&W64{7I>g8yNB2FUOFhY2u_< zEx9tdV{}dTnVMeId9%*hRF|MOgnpG6$C8*R8H#E(WO~(QWc}G;tSG#NNCkDoT(JoG z0!7~tLIx2bbvp8B_;7TEJAHMD?Pt1D>+QUG`i9Lnm;%@2+K#o&*^ncBu2`4-F)@rk zy^Y?_+l;U$2pO@{+6Z zasA)bKbl^PD%;5`=7*PV zCHA7X0?BGsNL+cAm^({Sv#%1gPnx>4#+LcryWF+l`V><>u6X1SYKa)xCGp#71uwBh zsoQ?Z=t_{8nz~mo6%s0gEQ8DE4&ifpv(Mf<(xRexVFzD4MRb%V4wqY2c-ITZl6Xf` zz8RP-67*)La~fH`{F5ST_wBR#tFE|>lWYBy4Fe9NzTF$Na^E7wSVgN9)>M^EBC)X2 z)#&0dLYhyXQ{z~>6`vJ{AV+)+$xKCDka8s0y^vx$5HbT1medV=N7?B^woQe!A|kE= z9uZ0nlT--%Gt$e6_;3xJawgqn;z$uZ9hSh&8M{ArJHN zmtWJl7o3Ui!uW|U55vBgOy*K_Uf`9axm9--@*oeyd}kQy=4%a!l{o%k)Xh%0X?d*S zoCtY`g_0GIc;buS$0dpp{%L`+xxo6=^oa?5;?sC!qUmQ&1GG$Yz(?~`JFKjAD$9$Z zv-AcZw5X4A+p4_ec`FRZ_vKcOK4+M>wg(xw2Dh|m;<5)Gr7rE`P7-ZtMf&A!4%%HMaa{8FAV#jurM|aF|Es_e>}$&RQpMD;8Z5^o3n( z%OMf_y0lW&)w=g)mw97xtr@UV=}r`gDt}L8^#QcV1aobRTQIVpENy zW6v}q8(+?lWt9+4ixXzFN={}^Oiw^8g1nx&yYo{TrqziXaLQ8*nSDbK_qC-&h~?Ab z6DO(foQJhX?^5f7>pQ&79!kDOpQzOvx*d&{bQ7P zTseL%TtAsOGjZwVS59E~5y1_n)Mbqw1D63#aZ*a{2Lr{{cI{1P*1QlFCI-c|b{gv} zTo;^|c3Fr4Je_B@ubSqf?Yg~elx6E^3*lqmH}IB6cXWuv`D<*z3GnI6O2}e;`Mz?u zJ!faVve*O&qU^qkZlrRfvoNmD`Jiftgh&(T{uU{-1@ANq&iGQ?rvC>oTiW)=DVkij zIZnGz@-PY7cD!j$p-xN&jiwXM9(j7iVwAld!si*XCSOJ|kZNHgDa#(!xo{QQiS6B= zs`g9-kR*Z!3mf(bbxq&mqLAZ8g3)_wmU`eA!*XEB?)@$Cv4>Rpj=lyxd?vZ7rs3oD z&F=}(a;(Kw9tmuq@V}YXrl*_%!F5biZ`Xikj_a>p;F?YwLgFG z5wnk@D7J@JAR#|WRFd+I;VcI*rE8g1;6ZO%1VZ@H3(SzuK5|=r?=Lvu2+$7rQ{9$o z-73AD==aHtmuxok`udfL^FU-9oJ9I=K}!dc>Cajll6-xgiQ2H3txx?YFHT*xF?}%% zz)M}|*}}{pHuh#HMnyE~)@b{sd;0Mn({NRBI37T=<;HXG=Q|ed8}_DDsj#XhL+)9qR$iq4pz_=L}!yqh$??S3-7M zW1k+*HQIob&)O1%L$hP()AOz9I;>;Mad#^AprBVlMX6bSnYW4rFUt1m(^m>f1P$UP z3)3?}>5BXd+S{c`Ynd2T2U6?d5CIuKDj80Ur)1j`#0fAgZ%N!>%SVWTDkLx=LE4N= zjx_ZvS6kA~^X>0Nvo}DHx^I}6Cp%05qH?_|-PJI=9*dM8m2qP&3m2WYVQ&&||R zoi_w^&~VlBzY`DlFXDkH#Vm2+hFyj|Amz5xEY4>AWyb zdbk|HR6tz_g+V#Hn-Kjt{4U`|jws=11G5t^3Kwj@TklL!*HNc-H!00t$;*-{R8(Ks zn0KirotsGZjg4?lLpVunPTvgQVVmnAK^CJ9C9kuzNfff)y~U_$hgpnGEueHJoQNzi zT<4+UN#Jx_azSjc5=I%xqC$js@9#V@JQSTv(E` zk)G?h6_Y~&?SOq_{N5Q`4x9_q4X00QH#uw!ZyKJq-y`S&N2&^Z9B)p9CAKYwryooZ?`}vu6G_8 z7P`y`>6$cxl;?4x>eW%*fW`&)%h%;Y+?{b8!R1_^)-Yb?6bi5V@AL|M$(Zoka)WqR zOY7EdYleWt3XT1^mHiI>;GjQRlf}xw=WRiHf&kC1-o zP26HaZLyxv5^`_g55z1m^Ehebazq zoG(%((XR?3F~wqlJ;|-3+HGNMcE;TiB=XkJqL4;oj6e zM}7wkTl><2JN+K=4gCo*!?yIg@!?mq_1>^~q$4DHbBZ_u8N_bCQA(^I(gM>i6j%bg zz%Y_m-}>My2T23c!a>2XTsXvV$<&D6P7{>5Nam()S!@Jn?+G$FklFjh=t%pysY5x{ znanxWi8dwkO72I)L|8+cDMP<7reITrzEQ*uc{4&uSW10I^Op}{87GO_!wHnF<{pEo z(5%<}DCKlOm8SwRUt~(euCB(;L-TlS@4vO8M{&zb;x@dl*$HHe-?o(;!U8X?lE1&c zG;&Pn7w8-VYZL73(>_-UmwCuqb`XM3m#Q%Ex}O*A3Dfeb!W~|bM?C(TXi|fUnVDxpalvFPjZcl+t>V8xQ*)|}G zFThu$@x5+28%IH!70iees@)NK1TSq#wI1Rg&bAR^Q##~Ea;UG$N|_WjB<^CDm};=L z7k+WD0O&iF?7tts+$I}^HMHzX4KmL}QJSk?@ zCvJu%gDwEh^d!7Jev@8L;+6Jcji|eKuC@f_?;plM@D)^4$Yo$iiG1A zR#S_)?Z(~hq3D9G>tWRZ-$L!KV#dgD?v}GB(%Ge6H7OfNhG&4BAas_uRD2V|zzASJ zStcfYzc);qoYPjEAMQx{GjBFpX-U@^RIxEG%f6(w@^5RM-VeC#-Vd8Q>vj1N;$))U zZfD2OMn|dIg6f(TuYN2|KHS|{oDF_*G!}d4Nl4YYs}}eDF*^C~Ht4maMwGGd^=6Z` zsq*pP>L`SR05kK+;!+Mk1q&$ zD5{hVzpmh@iTHNy;4CiF7Us$m><8<_DA6Kdx>n+dg?g$a*t zXAi%OIoo?ANzY`$7h2q^NRe_i5l!I6E6Cw~Jh~#MB0o6P*qwKe2@h5h)7%l0wSV4D z|NT=Cj=V-52`@rtte;RwIoXokinRTNuf8M1j{pTFDE--$m{~kg_18B&r3oO4;)<8Y z2D)@&R33xW6&o!tf{HDAp6<%YC{m1*!Q5zP`r}Bq|7h`xiPIop!d*E%X$upqrWUw1 zX6Uv#Ois5_z`G4v4gvOqGgv5mV_J#wj=Hg)X6zmMBt1P8YX>eD*3f+9l{m|IbA0tf zXx%|9egTN10m>;5P8KR$Pyl79mnp1uBoFiU`Ex?Rhl^m_@Dj<=bUQXpcFl zA2FY`qLSTxV>xRx^gng?(vQwnRQ0j{pWh4ibvoF-~zYdU|1 zH)c(;w0T26&8=stkYkG7*cnkq`m5S0h?Uc9e@a!`s<}JMjA7k}vY9w=5u;F9GII!) zwN^+(AzyBtTy1Y#2vJo1$^omGbByr~+lP{=MP|sCNL>QK1eU6Dk~L*1 z@Dn5Qxva^zGZKPE$T;|k&gVUH*_>&_dc`X>cIJ9~O}gQj`rR*GYlw1y-zrU!6WI-m z1fMGJ8UyPH<71@g>^^6|jiFyGBCfv4FT;!=;D2M4H=^*>P`)7wWbOwx#;Y<V!va&iB)v=l|7~JI73Aos;3T}ajcC@cJ znKn#cni}!lE4_>8~Wwc{p#tyZU*5jV`K&|txIoD08FtwyE zqpljOoQJDb)is&x0vUzCB}tTIynK-a6Cz9=aOn3&GDP>1@$vIOKmDnfhe!K|CCuTE z?`o34n$i9s7TxnE9tHtI4eW_pn9=VpjppOvw)xD~(;e-~#mP-4%+NE6`NtCE^n2r6 zfzM)VL5tB}PDAj>B{fYxYGF{z6Uh7U1$GRI)pr-@E#O6OpRv7?+-iAUNMNHmX|rD& z<&x^^a9#5$q4#36y+HPSiudP?gazaI&!YdXVeSKRz!FKKLj zVpFn%?YSF*(j% zQRCXExiNdp2dq)i=`nvC$R-%{DFO6S_q$$Y-BNu!+J)$;%4lmI!+X&X=!IHS7yD-A zh?Bn%meMZdoaDmEqy?)opE;cDahxh15Pet!=CPV~m&4m9El!XYkMI}t&B)uKL~w+? zrqkw3vMpXz3_exFNgg{f^4Lg9bNTF)mb}39T#=Q+!$25k3!kMWFSe|GJ*LeQG^CFs))y%5QjW%1W^4NZzpW<5u^(d*hz-pT4V5?2R<@qSG%=65^Q?JanDd&4f z%j&K4OxR}-hO+Sp{8K3>Exq)4=j0qNzzlPmsLM%M@ePh{vr$8uj!7lMnCZZ?3G-|e z`hvnPw;Vy$Nhd>yRX>K68(nEOtCPY)M>@aTz~Vs*B>3>0`PoZv(j@1&Lxb3{FkQUp z`BO3X0RI76$H=Z%&-?+vqHO;nb&=UxV{(aQqV-)s)iweWUUNQ+7mQ{d{gWzCC|uxP z`4_z9u1)x%5Ed%e9jca*uhOV5i|}3LP!Ue(zI*3CCeQDJaTF!9jK*A}=YlPYV&tB2 z6@;$BEylu^V$`D;CHDx^EmCiRFW4^gZ7~qx)^}b6F64@|G-7Yx*49e}MRWJex8rY5 zZsZg*MQRe@uO-`*ib<@79V(?x9O6B3xLw>SAPTJVm)V55zl>sSTdBP-Hu^x#&BvC$ zFg~`+J(G|!kaB+|ns00TL_;e^?Rs`ivz>(C=m}=l$#LDO~~vt={7} zs&qrZv^eAX8t$=l+5597LCI3n=e|@Q&Pg(_X!GxCVvNY9g-3E=X1oBt=F+E`JGKO_ zut&t9`M&k)#vuvupFdrT;2=RJ14(w#33 z3bZL$c0lWEiwbXZIuBDmU0txB9}F4Ez2cBKVEzRWMK=*8-)u^Q++mm&e?>sT{mGfvI!w(YIns|p>p26BX}3hRJY|ce)l8O~lZTu4 z`F&m>eLmfMmYaOCmC;+_^-PBKDrdZdws(pHhM&Hxe~8PfM^P0!5Mic>KTZE&Gh61K zHS14Gs0w+;U$(0DalZd5+My|lwWyP|r9&!PjmeNBEY+PkEj75ZxIB|uaF({Vsa|Hl zu`a9`T^F;Wxxs?0_-hU;hq|HW1+IQP63U2zn>bj`2@yA=M;Egkq~==EFHWR}C778P zIAYc*_!$;F54*#<55xS{a~!BzVBDG_wFbhJDL7a;{-k=%snMVHXoM*flvUvg z*B_L_5*}gS%S3n!9^Wf;ouRW*oZ~2EJYpG0Tm8*sFGL9Xo|okn$t1h zSS`l4Wu>7_mrG1-ltGBF7pZI&#tS;dIgo=XmrTp?R)50lNWalrZ4-cLT6pe*7)gj* z8o^6#&jQiys~dxxsSh1=zG8el?y1HgFyUa7hc_(yZO6LY9%?;5-iU?<7ZB0AHc30> z(K#m}L1R+SF6JuW_TnE6PjkDFUmdC?GiaE^hPE)Eno@IdbaLgeaC8Cv)DX1A{@>08 z)P#H?l0UsK{D}Sm>7#HxCw~J>O38vvj8Y%CZRTQ0t{|*F%kFU%mZTQ^+&lMZDWcfq z?HE?=S%*YM-U}vFxL^^rwkKhWMLTpRS4~4aDNNlXijvXsrc)xV9g3dHUWI0r<;3-D zU7qm;=v(g5uV!a)EqXmD(@_kiKR9XF3|HKKHZbIQ7;X%tk+RYa1_kSd8|0W^$+$_O zic#*hvkM1)Ayj;wQ_*b(+;~@oz&8F4sm1bhQkdxMp$fwam%*_Z=^~N_dVi5ku+>MK zr;otS%9P0!t$V44x{bhE8K)6GPJ{1u;}@XdXCw#5idOq0<6atp&Ei(77>tf6{SlVX zo|^06-kA?0@+8W%@ndG23OV^H!U*+~)hn~j#HS2A!~EkQHjBz`xp|nUm9h%@C@xe;t_+ zNyjBl^spni3kviV62S#E^q`77d4aR{W-yROYpo)S+(z2SiZ^F-361J=dRPeFG}q+W z;)ySG1{;TkM59|RcP0f`_dg*L$vQkj7Moq_=bwj17#_-uM!JtV+ycf2SOKz`o%QC|@ zgDvFX`q)1p8ogr1JvOc`{EFk8!djzCP5t5&oU87_&Rnz<=S~F72t!3fr6_0jebi+)N+d%BlFxS5Je8gkN2EHgUYH~ukRc!0IQ?_ruINvGb zp1Piyk)WTfSqtRG4ZPD$D_ffoUpHEgAdx<9`F@(aI72TvJebbTh4Gnwlgz{P!pO_xcZ0hiXcH z2l)G_)4v6Ot{qUn_{-qa?}EP%bo?ob4Sm+%#yfr&{(BqyPf-9s7xkC$|I*a{9q0F^ z=ATFq^#At||Iy<79p(29#h)l>*#C&~t6%Xu%J1dRKT*!0`4v