From 223a36853e9e7f9a371e052e7658a58185fbbcc9 Mon Sep 17 00:00:00 2001 From: Bill Blume Date: Thu, 31 May 2018 15:25:27 -0700 Subject: [PATCH] Changes to fix #523 based on comments. Made the following changes to fix PHPOffice/PhpSpreadsheet#523 based on code-review comments. * Updating workbook attributes is now done via a loop. * Unit test no longer requires an input xlsx document. --- CHANGELOG.md | 2 +- src/PhpSpreadsheet/Writer/Xlsx/Workbook.php | 54 +++++++----------- .../Writer/Xlsx/HiddenTabsTest.php | 50 +++++++++++++--- tests/data/Writer/XLSX/hidden_tabs.xlsx | Bin 9255 -> 0 bytes 4 files changed, 65 insertions(+), 41 deletions(-) delete mode 100644 tests/data/Writer/XLSX/hidden_tabs.xlsx diff --git a/CHANGELOG.md b/CHANGELOG.md index f2c5b12128..2300a2a518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Support to read Xlsm templates with form elements, macros, printer settings, protected elements and back compatibility drawing, and save result without losing important elements of document - [#435](https://github.com/PHPOffice/PhpSpreadsheet/issues/435) - Expose sheet title maximum length as `Worksheet::SHEET_TITLE_MAXIMUM_LENGTH` - [#482](https://github.com/PHPOffice/PhpSpreadsheet/issues/482) - Allow escape character to be set in CSV reader – [#492](https://github.com/PHPOffice/PhpSpreadsheet/issues/492) -- Preserve workbook bookview attributes on update - [#523](https://github.com/PHPOffice/PhpSpreadsheet/issues/523) +- Preserve Xlsx workbook attributes on update - [#523](https://github.com/PHPOffice/PhpSpreadsheet/issues/523) ### Fixed diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php index 43811663fb..72f1e5bec0 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Workbook.php @@ -12,6 +12,24 @@ class Workbook extends WriterPart { + /** + * Constant associative array listing the bookview attributes to write. + * Keys are the atttribute names and values are the default value for + * those attributes. + * + * @var array + */ + private static $bookViewAttributes = [ + 'autoFilterDateGrouping' => '1', + 'firstSheet' => '0', + 'minimized' => '0', + 'showHorizontalScroll' => '1', + 'showSheetTabs' => '1', + 'showVerticalScroll' => '1', + 'tabRatio' => '600', + 'visibility' => 'visible', + ]; + /** * Write workbook to XML format. * @@ -117,38 +135,10 @@ private function writeBookViews(XMLWriter $objWriter, Spreadsheet $spreadsheet) $objWriter->startElement('workbookView'); $objWriter->writeAttribute('activeTab', $spreadsheet->getActiveSheetIndex()); - $objWriter->writeAttribute( - 'autoFilterDateGrouping', - $spreadsheet->getWorkbookViewAttribute('autoFilterDateGrouping', '1') - ); - $objWriter->writeAttribute( - 'firstSheet', - $spreadsheet->getWorkbookViewAttribute('firstSheet', '0') - ); - $objWriter->writeAttribute( - 'minimized', - $spreadsheet->getWorkbookViewAttribute('minimized', '0') - ); - $objWriter->writeAttribute( - 'showHorizontalScroll', - $spreadsheet->getWorkbookViewAttribute('showHorizontalScroll', '1') - ); - $objWriter->writeAttribute( - 'showSheetTabs', - $spreadsheet->getWorkbookViewAttribute('showSheetTabs', '1') - ); - $objWriter->writeAttribute( - 'showVerticalScroll', - $spreadsheet->getWorkbookViewAttribute('showVerticalScroll', '1') - ); - $objWriter->writeAttribute( - 'tabRatio', - $spreadsheet->getWorkbookViewAttribute('tabRatio', '600') - ); - $objWriter->writeAttribute( - 'visibility', - $spreadsheet->getWorkbookViewAttribute('visibility', 'visible') - ); + + foreach (self::$bookViewAttributes as $attribute => $defaultValue) { + $objWriter->writeAttribute($attribute, $spreadsheet->getWorkbookViewAttribute($attribute, $defaultValue)); + } $objWriter->endElement(); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php index 842630356b..7c741252c7 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/HiddenTabsTest.php @@ -3,30 +3,64 @@ namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx; use PhpOffice\PhpSpreadsheet\Settings; +use PhpOffice\PhpSpreadsheet\Spreadsheet; use PHPUnit\Framework\TestCase; class HiddenTabsTest extends TestCase { + /** + * Copy of PhpOffice\PhpSpreadsheet\Writer\Xlsx\Workbook::$bookViewAttributes + * except that the values are purposefully set to values different from the + * default values. + * + * @var array + */ + private static $bookViewAttributes = [ + 'autoFilterDateGrouping' => '0', + 'firstSheet' => '1', + 'minimized' => '1', + 'showHorizontalScroll' => '0', + 'showSheetTabs' => '0', + 'showVerticalScroll' => '0', + 'tabRatio' => '601', + 'visibility' => 'hidden', + ]; + /** * Test that the worksheet tabs remain hidden when reading and writing a XLSX document * with hidden worksheets tabs. */ public function testUpdateWithHiddenTabs() { - $sourceFilename = __DIR__ . '/../../../data/Writer/XLSX/hidden_tabs.xlsx'; + // Create a dummy workbook with two worksheets + $workbook = new Spreadsheet(); + $worksheet1 = $workbook->getActiveSheet(); + $worksheet1->setTitle('Tweedledee'); + $worksheet1->setCellValue('A1', 1); + $worksheet2 = $workbook->createSheet(); + $worksheet2->setTitle('Tweeldedum'); + $worksheet2->setCellValue('A1', 2); + + // Set the workbook bookbiews to non-default values + foreach (self::$bookViewAttributes as $attr => $value) { + $workbook->setWorkbookViewAttribute($attr, $value); + } + Settings::setLibXmlLoaderOptions(null); // reset to default options - $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx(); - $excel = $reader->load($sourceFilename); - $targetFilename = tempnam(sys_get_temp_dir(), 'tst'); - $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($excel); + $targetFilename = tempnam(sys_get_temp_dir(), 'xlsx'); + $writer = new \PhpOffice\PhpSpreadsheet\Writer\Xlsx($workbook); $writer->save($targetFilename); try { - $excel2 = $reader->load($targetFilename); - $this->assertEquals('0', $excel2->getWorkbookViewAttribute('showSheetTabs')); + $reader = new \PhpOffice\PhpSpreadsheet\Reader\Xlsx(); + $workbook2 = $reader->load($targetFilename); + + foreach (self::$bookViewAttributes as $attr => $value) { + $this->assertEquals($value, $workbook2->getWorkbookViewAttribute($attr)); + } } finally { - @unlink($targetFilename); + unlink($targetFilename); } } } diff --git a/tests/data/Writer/XLSX/hidden_tabs.xlsx b/tests/data/Writer/XLSX/hidden_tabs.xlsx deleted file mode 100644 index b81f8b64b88c55b2d364507df654c283aa0c032c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 9255 zcmeHN^;aC()@>}f2A2TA-3dQ;3jiDd03Zcm;I6J1K>z@_FaW?y034*2h_#gi z$jU)i+0_PQ|DMss(t;!x8j?C2015v8|BnCS8R(0juX@glw8T4B5Xii$E{65GCO4Vu2#T{k@m1A?o1Kzz4bzlQufB&izm%?VWnt z3t*dk)&*}2Db1&ZOBAVU2+Xai%UPQicpp4tUV1TE%#(t?k|1&Y4JfYFl-qgg%{i_l zAr(XpSnmBxE6@&+Z(h14G7G*yj|>9?NZ$OhjUrS}(Fk>^>uV_{EwnkUm<+9&+>_m~ zGYX7xA#Ry$7r_8p%}mb9?UG*7^+-XexCp0WjE-_IuASx1r>Jn$Q=ESN7!)bNv=oH8 z%0y(f>@GAR(=v#nYB9}lalFPof%Pmu#liw_{pc$X$s5s+-Ga7gYrN41u3j{;pJV*V zEa}$^yuC5_ceP95?jm11@ApG^nGtWU-}8nFkT}}~%#>DH&vNpj(nH4}dAu>7D|m9+ zrm=Ba$jM;CcL;8jvA6sIrr*;O6hPr`6x$B+f-)6jmGhF-L!1tC8eINDNmD zJ^&8F#e(T~o;X|EnHyMJoByg|e{co@T*$y#{=1L1*fFy{W~6}Azz+YZcH7uTg%KWi zn!xskC0OQaj9^&{!(u_#`_%Xo&Z_*R$vkrQ3zp9x?zw+#5|KHs(ookBLboiYjn^aS zT+6FnlZ4c>wOpfMMZBvjXJq1>m^YV`H)?(Rx|j>7m(uGWudGGL4id9~sr*MKF1Whd3OH7?=H#p~lbj zK?hp{62>(%M>}q#cMwWLRKS){jr@~zGqlnJ+h+sTM)RKNS@bc)L1h7+0yZrs3j11G zq$tnByJ|Z2F~KW4)u%(me?!jnJUcx$7&SPs001f&5@6)~6*FZj@8y@6F+H>Dp3rXj zx#Z&5L*FS%hXwUfLc_eGdSe?J_|mvRBpmI@bB=~43l&MV-ic^waBtbYqSm{rIM9AH zez#^AZQ&xiH&r;Xt$BR1e{(1@CJ)(Ciq@!?)nSmGC)wkvp)>LD1IUe1xD*?A@#c^R`((V&Z6SsfJ+# zNf^ADPe+AIDQsQK4V<>@tn6G%V40SWx?T6!d(^P>mUmH=1jYM6pJQbBj7UFgXu(aP zEmW%RFKKyZI@HBGBgYWztFc~rSuV-ZIHdD!k==vQkY)1>joi!Cz$q#XG2XI$_y`wW zaWcK~RXQ5z(>Rljk7-eq_jHW&J6~H+ESGfa8d5NZ(3R6%hmq;n*n@JkYjIGO=f-&$ z^%7lj1chB{YA=fIq#ANFm=T%J0jP+1)Mp~C7!fR3LB&D%PSgZY0v=y3RVFUygA<^G zd5w?d2eJHnAiaH6O-y5^3j_)Z?RY3y1DM!D2G=#KF%?j`l8(gRQL{XJPWxd=`ryxN zs!w4!*|t;v^GQ>1Qt4p5ODT`=#ydUhE%x{U!!|Ulu0Y~33pD?uCo$zJm%#bJ$q(v~ zl_|_r@?MjMzsX!C8;7UlOzmeBXF0HaMq~Qsa_jKeetlzW z`BZ|p`>G7+p7-9a;j8?|DiH}aJ5MX?3Yh$CoVw;sS(-UK1RqAYHiChTA^(k*toF@< z;x4{w<0Ol$pL!21kmv#APNbOr6lpp49*GjQF2}79XQY(2Xf0>#nau*E7tB#x)LHlZ zk`ECe?+nPL4(HSkXl(NVscxx(^^=Bt%Sm`UWwQyzL5gbN+S1hQgi{xCs z2BV=xIuYc?;{%WPa!xvgJPlJ`p>lgO_7f}i!09wOggV0kyK+{-_kp-9Du-dEvO%2! zSlviuM@1z?lVKp23HpxVwNB~sCB&;H&}UK~1rpxSNz!GHQLM8@th6}K4v(w zCZR1RW(HqU3}aXCUq@{hOsUSDAlQUfnCsLeIqVt9nYfBWZqJd5!NYHbFY;bU<-@aB z47Pi4$(xs_ixpTUK(Y81S$A)09GsSX&uX&|8Bw~2`c?#xkC4nv)HLxYdzHThez#Y=?wBiMZJBtSEg#Z zRb6gR601)-67?q|VZOVACskcNXAWRFeST{tE7JAQn++3TQds*g0?t?75uAFRc-W4Y z6apk4O?0CL3M6z+k-V?wRJyhaNhVu%dTcLYdwet;_pk$WAoePC*3y-*_eJ~777?x; z?vgiZe}<~GC7jV5BCeUvi5T5j2)Y#H7Eg5ZT4}wZ;61uFJ}~R9VA(PwCe`ZM zjm7F^(yIxvHovb>?dOZ=UDC~iT}Phl^sYR|Wc=EL;6na6XI>;U%2+o@BF)!JFz>DV zXU|viy-|{wFZvyEVd7{qU}_H>F)$@DD+}Xx-UG?WPKN2SLZd~ezOTES5BJy(L#s)Y ze?*9EC&93I0zz}s3XOZexV8RQJiDmw%^m%c8Q`*j`)5h8Hvxeh?3sQWSbsq~Jx)3t z4DG?wKrh&wR)!_l#d)0uNs(&>btz(Fu|!LBX+!cIu>8M16w`1;@umoOTlbXez)ik4Sf^b>ewhV%@;ig5x9G>?^*}d2=q-dr zw>9Gk?kTCU_i*j(E0Fa>cC{cKxD?L-2*>gOZU1?5(+3y> z&>;VzbNS&)LSa7{*Vs3!HCRZ8sF3o*c)=(Isk$K@e5gcg-iUil`DPNTaI$Xh<`%r` zQ$AA)dafeR5F@844`Qzo;6Q(*1M!ncvdc&rUlFOVbnQ$|Tbt!HKQ~%)KJYeIqT|6D%N* z0QQ7DMbt?a2cDTNVif_3&ZGppHptoejUhvaX3dUVMfQCG?V7=q;h0*@?)|v8{}xVh z>esCkLJm~PiINLV9^cuZb>`+`b^D3Y<(?S5O@rguOjJuWcURIo56{G-?5&e*0 z*5L(+0)+H;Enywm?yEzU%bX)z)5C5Ko!NalKg38Y_<#5p^+&`qxS-yd)z6z?WQdV^ zc_Ob@4t!^6{L1hIUe^A(;C17mb;yDB!T`xHzvMT~;9vr>1Tp<~X8DDUV~w$Jf;X6* zICp|b_RbF+TTv9N>k}4nE96EQ@r3ma$BL?Ktg&qbFCi(oFM-tg1xdoz{PELop^)2u zM8T8S9S~1Zjm?vl?~tciNNGADhf6%RfBJNt>E(9ea5mkZ>?VqCS@9ADV3W_0pG_$vakkb*8GI`dZFqBz$+Jn zzG0b$2fFOpHzZ7%{O*t64=#c``MrPa-}dU(G}qHgTftH&DV@wl=|27ZiLiNpj1EESXptQDI97e@St3*)fNA6FG+$^TK5Hibm{KWT z4YO<*;e_kVvtE)q`u)32rp!v*MRRCt=wyuf38PdQ;ChcOTWu4#5LpG!AtGD9I^r$+%~tP?Q^8SMd)1T3D9ty)(07MaiT zdl-@FgYSdcE2Ma5VAOF6tIWb8%-#A0WNU6MTs; z)QNz!g>wJueIIA6wB7!6b2u(r48PNSqynG)qD<^ipBE(xsaTA&za)-F)B>+4zX+j1 z4a;&Pd_&LA53ieGw#`!GsY32R<3!bM5h?A&I1YYQ;h;snhWYxjP&Qtki{gaER$L{A zhyh&pMge>ENqx4_z2+&k*Ln#=$5Lf3R^L*b?)vG8|pB>ww1dFvBCcYP=dlQ#AME(+`3}K*Cw^3&DES7R@Ntv^a@Rn@{6Jg~Ak9 zZGRcJ9oat-T0`~VJo3sJ!u|zJl-AjOJWm=mXiwC)7XF%;-rUV3bGgeVw#@#ktBSqO z?9J1C$%h#oflAQ0dCG<6z~$AYkXcHbhwljG>G^)9d|UMn8)^K>C`^B=u(Kz0+f)QB zbJ3Dw3stij#Q5u*RE}i&<#;lNEn7-KD%QoQNhD1X&`MW&n9QZJ3mav&VScb99ah$4 z`MunQOH|~HoN;qmOKLG9Ozf=-jdr9+l2qZoCjaSb`IjTFePw5jq!-i+Gr0OP9|#m3 zaTRq@gfVrD19TQr)TbQOrxXs8ax}$lcMzB@v9MfruGYoccY=i$g@Qg1#_t(oeX2`o zi>N>tPZ4*EV=GR9P%vO(BZ+cQT4=F#HFGpAX&1|9Ad7IQjbxr?_QTnr=N(JD*BEF? z3I4(=-)zuKV8?bCiHdG6_L{26my%LxvX9s5w%!b`k(P|z{PyKOZttASi)OMfS~$5% zO-^`{nN~N>`iXcrHNm3sJlb(`BQIEDlHKF`7<3uD6Oh@b2y-pz)O^qcG`CXU(h;ZG zqB-ymxv+MVnFg)kDrU+N`)(*z*gTf>uMw4zFA(;$6Guk1L-FjrLC;rLXQG4s80r)5 zZ+AGx;+s~56|-~eys0;0GS?P1%^m-dj3->dDL2_Dvxg(J;`5X|9Wk!_LTY!;5hGu6 z3Di}){K^~R*%A8}rn|<4{j`T)9O4dwdAE^E&qcn}&=y$zlw+`~FlTH=!I*WGYTNkY z84lIW%#ih&RaT=-t~acQ+tF#XtX;IhpVidJeW{&P4wDl*&oJ=bR=NM16b{NJ2(Zj=KbU)+N zZ`^HmXfJr!o}!av+}Dp}Ygjs&1%9aKr-p8q%bPs@$`hzlu62s+wo)}k$oIXS3>jr{ z)=J^%4MM$LqZXVQ1;{H;qX;%Zinpkd;Ai$v5;{Y@9U4!g4#2fpYs1sOnmsmF4<3Zz z!W;+v|LU{fYH)hIY*ZRETF`029Wnh{sP9-TGz4AZafy48^5uoaXMid|Y3Wv7<>x8U zTT+!7kvfdTfvcMoSAN6XV5OyKC=)Uj93|@wAY_I@;BiT^*Ztjb84XLKGmNz|EQ=?M z2yyK7X>Q&$uM(8HU%+@@T|)yZ9$RiuVp`x~PHzVAAWOP>v5y z{8K}G+kMIoSm9d-z_3{~F%q4C&}Vdo8jv4A(cHyJcPOb*7b^J@(V9Tvq(h=WCKjHggR}8RTD=e}pcLGEK`wwH`*CdOYnW$M81~yANS4I*e{Y z=hZ8LgpK{MoPDH9csI^c80pXhD; zL=YfbRMn3j&M7d}YboDxiyKUSC(qzGEayg$BOkly?dsos+Wknje>T6ia^-BiIQj_I ztN(m{6U+p75%9Bvz^ph6;ApHug-0)^?y@au(ca_@8SE21Z1zqI@q2X5gCa z11j<<18Bv8)G|q?mMSTkEAK3_YegJ-5m39-nbc70l!7W?H?gsmPH^6@Sh|l`7MDPS zCJDs!-AIW3pq4#1Mw^*8_c) z&A`V~A+{vwVEE03n$%mHI$mwI$6+TQDpR*$4j0N~<0hDa%eSJ&`Rh^BnhDGLn6}f~(yZgXn;Z7$+FOM^B&cWP@z>h16DLmED6Lp@* z&u67tl%8+&>QgNjS(=0Jr)%~!@46`6WowZuD9?}Itv4ekY4Fg!0ichS0^+w|Yv`ZI zBXAffNChKL0Q{|u_E+TT+t~aMcwof+=SYw1wp#kt;(@wC2|9^fwxAArtICp3;HF*; zkZi4`oT*mOU=TW_3tqu0=1IUD^bJ|Ql1sNPS9qxOlB~D#=+|=;aBbCkWHC&G4Z)zP<87uXv zD_$9uA!1gKcMvkTQ0#H`r)yOOZ|~>ZQ|uKlYHsO|N$iifKvOWmT*4I+uMIQ>Xgby% zb^&GZTGsfw3WmJ!174M(Wk`-g8+5I5Qi>dF<>!0%{Rra#Q!}auVY>vmkKImA=^gcK~@NkyH zQ(+hGry*9}ET3%plXA&Fv=lYeODdel7DmdzTn_chby(L_$j*znUbSJ}C7rW+!Fswk z4he3z-D$`UkM_=agWt^Wc!q#v09TEF-%Rnx`}<@3m#q~F(tl6z_lEBufxpHKFiHN> z{(T