From 6ad5fc5fa6d83370a3f6e75185210ab44abc0949 Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Sat, 5 May 2018 21:41:46 +0300 Subject: [PATCH 1/5] misleading comment: not every sheet is examined In fact, if there are no sheet-local names defined, then the loop will not even be entered. Only the sheets that actually contain the desired item will be iterated through. Change-Id: I1fa94d8b9190f43b2896a002acdec18397225395 --- sc/source/filter/excel/xename.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sc/source/filter/excel/xename.cxx b/sc/source/filter/excel/xename.cxx index 4f9963b..84ab353e 100644 --- a/sc/source/filter/excel/xename.cxx +++ b/sc/source/filter/excel/xename.cxx @@ -645,7 +645,7 @@ void XclExpNameManagerImpl::CreateUserNames() if (!FindNamedExpIndex(SCTAB_GLOBAL, itr->second->GetIndex())) CreateName(SCTAB_GLOBAL, *itr->second); } - //look at every sheet for local range names + //look at sheets containing local range names ScRangeName::TabNameCopyMap rLocalNames; GetDoc().GetAllTabRangeNames(rLocalNames); ScRangeName::TabNameCopyMap::iterator tabIt = rLocalNames.begin(), tabItEnd = rLocalNames.end(); -- 2.7.4 From e2daf15b155edfdc098520a821c71b2de90f3596 Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Sat, 5 May 2018 22:03:34 +0300 Subject: [PATCH 2/5] sc excel export: use name, not index for named ranges map prep work for tdf#113991. The index that was being used for the map was a LO implementation value, not data that is used in the excel filter. That hindered doing creative things to overcome excel limitations. The name/tab combination is unique, and both parts are obviously critical to proper exporting, so use that combination instead. This will help to emulate global named ranges that are relative. That can be emulated by adding an absolute range to each tab. There IS no index in this case, and an existing, conflicting name will nicely prevent the lower-priority emulation item from being inserted. Change-Id: I0bb75b0aa8a5d784754988f9428c5f2b3eee0da3 --- sc/source/filter/excel/xename.cxx | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/sc/source/filter/excel/xename.cxx b/sc/source/filter/excel/xename.cxx index 84ab353e..2b6d991 100644 --- a/sc/source/filter/excel/xename.cxx +++ b/sc/source/filter/excel/xename.cxx @@ -143,7 +143,7 @@ private: typedef XclExpRecordList< XclExpName > XclExpNameList; typedef XclExpNameList::RecordRefType XclExpNameRef; - typedef ::std::map< ::std::pair, sal_uInt16> NamedExpIndexMap; + typedef ::std::map< ::std::pair, sal_uInt16> NamedExpMap; private: /** @@ -152,7 +152,7 @@ private: * * @return excel's name index. */ - sal_uInt16 FindNamedExpIndex( SCTAB nTab, sal_uInt16 nScIdx ); + sal_uInt16 FindNamedExp( SCTAB nTab, OUString sName ); /** Returns the index of an existing built-in NAME record with the passed definition, otherwise 0. */ sal_uInt16 FindBuiltInNameIdx( const OUString& rName, @@ -178,7 +178,7 @@ private: * -1 as their table index, whereas sheet-local names have 0-based table * index. */ - NamedExpIndexMap maNamedExpMap; + NamedExpMap maNamedExpMap; XclExpNameList maNameList; /// List of NAME records. size_t mnFirstUserIdx; /// List index of first user-defined NAME record. }; @@ -349,17 +349,18 @@ void XclExpNameManagerImpl::Initialize() sal_uInt16 XclExpNameManagerImpl::InsertName( SCTAB nTab, sal_uInt16 nScNameIdx ) { - sal_uInt16 nNameIdx = FindNamedExpIndex( nTab, nScNameIdx ); - if (nNameIdx) - return nNameIdx; - + sal_uInt16 nNameIdx = 0; const ScRangeData* pData = nullptr; ScRangeName* pRN = (nTab == SCTAB_GLOBAL) ? GetDoc().GetRangeName() : GetDoc().GetRangeName(nTab); if (pRN) pData = pRN->findByIndex(nScNameIdx); if (pData) - nNameIdx = CreateName(nTab, *pData); + { + nNameIdx = FindNamedExp( nTab, pData->GetName() ); + if (!nNameIdx) + nNameIdx = CreateName(nTab, *pData); + } return nNameIdx; } @@ -463,10 +464,10 @@ void XclExpNameManagerImpl::SaveXml( XclExpXmlStream& rStrm ) // private -------------------------------------------------------------------- -sal_uInt16 XclExpNameManagerImpl::FindNamedExpIndex( SCTAB nTab, sal_uInt16 nScIdx ) +sal_uInt16 XclExpNameManagerImpl::FindNamedExp( SCTAB nTab, OUString sName ) { - NamedExpIndexMap::key_type key = NamedExpIndexMap::key_type(nTab, nScIdx); - NamedExpIndexMap::const_iterator itr = maNamedExpMap.find(key); + NamedExpMap::key_type key = NamedExpMap::key_type(nTab, sName); + NamedExpMap::const_iterator itr = maNamedExpMap.find(key); return (itr == maNamedExpMap.end()) ? 0 : itr->second; } @@ -536,7 +537,7 @@ sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRa xName->SetLocalTab(nTab); sal_uInt16 nNameIdx = Append( xName ); // store the index of the NAME record in the lookup map - NamedExpIndexMap::key_type key = NamedExpIndexMap::key_type(nTab, rRangeData.GetIndex()); + NamedExpMap::key_type key = NamedExpMap::key_type(nTab, rRangeData.GetName()); maNamedExpMap[key] = nNameIdx; /* Create the definition formula. @@ -563,7 +564,7 @@ sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRa while( maNameList.GetSize() > nOldListSize ) maNameList.RemoveRecord( maNameList.GetSize() - 1 ); // use index of the found built-in NAME record - key = NamedExpIndexMap::key_type(nTab, rRangeData.GetIndex()); + key = NamedExpMap::key_type(nTab, rRangeData.GetName()); maNamedExpMap[key] = nNameIdx = nBuiltInIdx; } } @@ -642,7 +643,7 @@ void XclExpNameManagerImpl::CreateUserNames() for (; itr != itrEnd; ++itr) { // skip definitions of shared formulas - if (!FindNamedExpIndex(SCTAB_GLOBAL, itr->second->GetIndex())) + if (!FindNamedExp(SCTAB_GLOBAL, itr->second->GetName())) CreateName(SCTAB_GLOBAL, *itr->second); } //look at sheets containing local range names @@ -656,7 +657,7 @@ void XclExpNameManagerImpl::CreateUserNames() for (; itr != itrEnd; ++itr) { // skip definitions of shared formulas - if (!FindNamedExpIndex(tabIt->first, itr->second->GetIndex())) + if (!FindNamedExp(tabIt->first, itr->second->GetName())) CreateName(tabIt->first, *itr->second); } } -- 2.7.4 From b183a47bcbb28cf5862011a26a571d7cc7f1dab4 Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Sat, 5 May 2018 19:39:59 +0300 Subject: [PATCH 3/5] tdf#113991 sc xls export: no relative sheet in named ranges The MS formats apparently require absolute sheet references in named ranges. So, local sheet references are now forced to become absolute during export. This patch only affects XLS format, since XLSX composes the range in an entirely different way (using msSymbol). Handling GLOBAL names will also need to be handled separately. Change-Id: Idd2bd841264fdbb30e861da8956da3d28158dfa3 --- .../data/ods/tdf113991_relativeNamedRanges.ods | Bin 0 -> 9069 bytes sc/qa/unit/subsequent_export-test.cxx | 18 +++++++++++++ sc/source/filter/excel/xename.cxx | 28 +++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 sc/qa/unit/data/ods/tdf113991_relativeNamedRanges.ods diff --git a/sc/qa/unit/data/ods/tdf113991_relativeNamedRanges.ods b/sc/qa/unit/data/ods/tdf113991_relativeNamedRanges.ods new file mode 100644 index 0000000000000000000000000000000000000000..c039a40917d84d8b763859d1ed9bf97f1ed4cbff GIT binary patch literal 9069 zcmd6Mby(C}*EU^BN=hgaf(S^L(kR`H#Lzj^(9ICi-Q5z>At~J^u#mdgcF3c+|B*McZEFvH%DE3xTMqEPt zt(2UYtg?i{dj&~BHAzVYSy44*NljI06-6ZtMTPh3vg&FY%IbQGnx-0>099S{_eLQ7 z_p-Vg8m1Z=7Wx|Y21+194JTu5BO?QVktM(wXkq{`H`X&YvjCVmn*nXD^>kefbv%r9 zKz2Y!Yg3S|t-XVzgQK&Xowc*Gv#Eohv3r=cN0^y+l)ER`);rQ8BnfN*@Upk^c6I*X zY8&BV`T^__>uDJSwoCH2Px;`G8swZ6;_B_==j#(3=;s|8n-m%7lNRNl5)~X56O)vb zSnsjjan$*wHUuPiMtt^8bD z-hDR)5ub5NpeedL2GSsPkljqV`Y0&LvKTQYs;6>j;hI? zhQHe=H{o}J=d)lT4TNb`HPmK(Yj*mgcN9Q3U zeURnB`R%c}HOTzR*z)e|%G&hG*4kA6+U(f!9Asm0Vs~YHdwFVabK(2i*wOaP%Es2l z&i>}w;_B|1EYt7O~wYJba%j>((Glz`l3nI>b{WWL^SZa><8+c1Lf(_|=i zTv$>njpO{3KBE&jfR;?K`DL{OgZCAg*YhTNZ5fa3CKqXWE~nwhp-y7EG@aXa=Qr1F zEEZ-W7m$pmu@|SQmRu8=JEj*7w$!7I^Ydk&H*-qT#iDiV=jZtbr_^h7oH3k_Z*$YQ z6Gg|P!z2V}x{e_fusgXfZrh6X-)!2(9*grk@A$w{wNlwrGET~c_qH!Qaov2OpyRt>%no) z&DFTKOF!SmSkI{X+Mbj!HRWl% z{cDplS0Y{4$|hlGyjo?ops{klt}OZ8cf#hMi}YK7NPQ+IJOGg(-?(s;Za^{$&Ub5$_k zRX*mC=<98YL#qska^A7h`NY{q^85k?=7igJ2RUVCdj^&b8ec>oYDo!Dn}&(P4vll! z4og_P9o*MgNqVep1xAtb-S=ZMAn{B=P|@ zQ93I#ucB6t2~u579%8nDd){Url_5;V$8Q47%Ke-M4$&0k=X_kxWVD{iIE<|FZ;}~o z2H?P!5&B8^^Oeu_enQ9D?a%d^*03 zoUtIeDao&ZKXSx_R3Uy0)4EI1=^M-d-RRI=ulu zL%~>G1(S6Q6PMUU&SX3SZuT!CB$s&i?2Q!crpgsNJ|VU-Ln~3otk2BBHH~0w@P5xX zSR*2z<4}u8uEF=*jZ5L0)={3_nMjLpz^H9PIOXQU1m%I6R!YKf37a*1{E;ZK&E4uH z+i59FFvm(l7#a4E(yi!XeGFeTH=W}fk=gg%iav7QsfTJ84jWuM?E8DDB!Sn&=Y?MJ zUcpi&TbmLT55g1sTzfoE>qo;G{ekOi7Y>*#Fhy7@qj}97*$a6HPs%O7P@%>aeU7Te zo&-(6wS&*jBr7SNpg;i3*ggQCwbN%;x9Ix98z|xR@s7`scucqTREww_9y=Z))EE;o zb+aH_A753I4yG?e)H(q<-EW6?EJNr;o(p;O-V&rF;Kvmcyw}53C-Zu@S>#^dL`5&@ zgk7uQwI+EGu)z%BXOAY}dJ~d5_Yw;0|Lt+$$fD$%SEJh$%-SNUR~QFg*SmtFLwxcF zc!?|9W$E;uji2LXzZizUq*&RF-IE+y|5h1*qVpgpBG~yhcD$J=qgRKUNY|1K@I{FT zs&ujCY>%kw1S?UnmL%!U1%Jtn$J_+8xu4hV+|Z9r zAH=ggHa3cH!q9wHu`OL}YS?80-DUmGY6S2Lovu-`u$pR0P%5EN8BCB4HqK3%_y)9C zIxU8OMKmW_r4H9QIGQp(*mRW8Zud!?lQd6ej|OM7hb`jlXp6JJQAtRj(;;>c`4i=q z_I4rL_)aa4Nx`DL=0hmWK_%{GM%!8gafz7^aB{fW@qYzhbzTqU2Gi&)w=bB_+ zD!H))DHtD!RJ^XLgR?Z_q%l6jdEy*uYPOZ-K~yzYMtruS7)p#+lB7%1btM+n2d(BS zPFI_pfUd{z%9{z*&y_tdSc2?Uvh0od#hQY+v(^o7ta6FYPF(c8`l&69_DoMYc!1!d zp-$2dPy2B8Qv6p5?SycbeCgeesjwEIVG$Y_u^tzBDqQ<~Rw1`AvJ%KBkuMZsz<0T` z$a9#x@qM=bJMw;TN8WmtK+t_E{ug&^D6FvKy6=~Ce@>9TOef^wZ=w1I(`TRSyVqYn zv!(H_Y*kou>QI!~%R^hNNqBHtfZS^&p#kBU+eWg?yNz703I|MD(pfzQls;;$SKQx` zID?3uO_?vdonOHwJMIsD;n5Ql)FmOuC!igq{GdiXKI9cSD6F_A-QaD64fg&(1E$4~ z2&RZ#DT)m=2kD!~&p1iS$))=tDA>puixw8x|&;20iarIuno$ zf3*f>^^Ytnbxfzc0I8Rps zY>kjX@a$bPpTg~R8oEwBt1%sQot=NV5|~$9&04)y5Fa&r9#1tLYZT0;SjC_4%@62k zWwjZ%K!2=D@7fa3KI`3Ow#1Zey<#0}&n2}4r%eviG)cZd8Da0UiH&k?QxE%Ktni{_ zCaK;rRN*s1R9=gxzF@2Q7Ev){#%cCD-Y%|{a%;(DE!S}iU-X(Zqpr=O@Z)#zT*1ptDZ?0{u-%q!WR+~pRt@bpysj@a7!Ey0ur(L;q)G2h30ee(V4kc zyr%hLHveef@1q}Jmgh4#p_f(ZgPF1T=1Th|?7HMtxi=j_qku$JCzxn3nnJM_>%lYO z2N92m3cZ~%xtNAGO7c#~ZsBiVPK=hnVu&X5ZQo)~zR`&!G6NSbxIdZIMCV;n6CmHG zf;=x}sdiuZq_R^QF3HA>1D}pBOfvZDBbe6;l3^iNC2@XgdEQc!!a(yapXjYVS#Nf8?QRnpNK5^e16c$gkVglSduB}MiuYZ_iD zXeNc4aIAHmUx$dEzV<~AcRIBS-6zBKl61j?0j6lxs>ZCxUcEtZOu4ua^=)GFit&Sa zfPCH}8c$BimQ*-CVRzSi@<+Yt$Lc%N}eny8~ftlfkvMXf#MDR8Y%k)&*w}RLRf(MXc*YL=L)nwR2 z&PwE^CX>kQnTwlw;?a2S=w$uqYPGQ-jzfy&L~`DwcU z4n{$k&$oA?F5ouv33tQjGA->c+`vc#PZSfpN0zF*!f)GlmWJ5+p9U9G0a+U!BSqYh zXdv*MLB`S?Z+zy4(k#*T*@x0rYM@uvbfA_8&>NxWJN_jTmto0gB#n>LG;uF(b(81b zjq?_q$W?_EOa>ODd`Svj_*UOTkJ?%9lwf{igEpIb)N-@>W&FL*qKG^T-4C zUIN5DMreBPB;XP4ze&Jf1sekp;GX|Oq9nqSUg7$lUgLG|;$Jl{0bncg>kmxK&rqoW-1OJte4^$wq2Kj#yW z>8S5lRH7H4q3~xh-brhTO^w>tdijY2Jq17a+m6=e;lsvK`FQ+eYV6a>_>kD%s(fe( zNu%lvT5y;3!_HT)=1@mjr4DB2WT`CQ^MD2L)KpJ;cG>F=cVCU-<#`3 z+vC5Xjyc&DM#(KaTQ7uZVdU;z8UL={2j?xVSa!A&?qNZs+zU05dJL6HUDPl<&9*D3 zrp;{A>U^D?vxbCa@ysE_lW`#3B9u@)5GT>F(056lwTre(nHq(F$h}JRYBP`a<}blt zg0H6k3=0Eeb65EKv*6#?$!tMR<_3T5)#lXYEN9rU+!so)6m=k;J<;D-LmD9hlf}f< z?1YK-E-#G(B5UO{`Q_|Kw7*bL^+p=d1}gA$=u-rA?}s`p%theF^cBv;tC>Ax?v1M5 z4S7HFEMN#eQP}o!#h+u)*RASoFVdeJ+VgVJoh(Y2JE1|3vVxzp$53Qrc2I^y&0`&GvOV5OB7~Sn%r29EQ+cCuhBZk zZhy1|!+zA-l_SoM$URl-KI20tU`e*55sC$S(y$@(@-xAC5VuHO4Kt*~%7h{QEh6u; z;EyfLt0AxTOPL}ZKf0l5aM1RwP+^1fTX1SJ6VW1J5oVQ~xFqE?b0f5OjIHcHWnm;< zb> zLry2ux)eoP%2F(DN2NERSx_*^Pl1i#lA$w*ATT93%xNahKJ?gDjqg?=TL5}qsM2g3 z(W*!Va}0d0fvwah(0xQn(_8!$Jew;lJMNo+pTx#G#^QGXuNZ44DkY?+o8yyUTKImt z(9ibyAW0&B*^S$n3koC7thgWZ5p%%(Ta~@SuHj7gFKh!ca5cI z2qG#mtqycCSeYH)CRQTm$n@ay&Nt)HB7u1yW`e9j!Meq4YzA0P^+C3K)q zoM?b9ES`fo+V}WDfh{~V=_K(g=yvdOpa9?(4~i4R;p@ybW2hw?aE5lJO$^ZOya}-J z6Up<^gwq~SAH#!|H$r%cO5kvolVg%zWYw~&5)`dG>NJO!DQk{s-s%-2Hwy^IwbvCP zNEdh@lj3QJ^W0QnJ>=ccilan^_(XL&dnLuWAg>;0fDMTNqZkN`xkif!IX&o{S6XT= z`VcjzI82z;#otsUDU<&d)KCX3RN7&tsJRZL3&-#fvKzNMlJ7FcJP+#A-I3qZd5leK zJ5c4ouk1uHDm+Aq-Hpha$qau8<#z!k8doAaYC`lD(vVMuM`WiINI$VMijb*My_Hh= z6o2!!W9)2`dc(ncJW33cOIe9xTrY5xvO0)2Wjwg88k$hZn6f#&9qbN^xKg1wF&*e1 zi@+`$Z;`~|^OjDPJ~kN<^JHffa=l7KknW@4x1WsEdWdm2_uam9f4&3Wm2n31z2hgIl4>|uk?{A?3kNz3c6 zU(gaR6Zr1ZL!azVqQ92fm?|4N;ur*jL2-tpk?Bn(k3gbiwR`91sXdGFP9P~8+E+Ym zvOCcI`r^>TNlkvM6G2jkK25cjWb;jh{4?iuaQZa~&W@~h zSmr8#xGk;;GnIG1{d5xuHFtQ>>{UM(St(?bp}#C#`~B-7WMraDhQWl#Jn0H$nyi$| zMShyPu~JG}N+Z3bn!h9~&CobavzMAYG(F&Z3S7CBn%$NGtqf0rE`?Pz?Bq#sEtBUW8~{Ei zMPtPmieqC~tVoIKv?Df08bcYDQAL+WM0SI+2JeC^#6{RFV?x9g7O;&Q;Z7vYeb0j= zu7P~?9!7M6XvfRj+uJz1?6LG)7^pk|6#;VwLR+5pmu3wSva^a=w>hq~meErk3Hz-q zE92AZ)PzdtBQ8?>^kn;4CvN9t75-D!K~u}p;U5m4l|2!+ANIuO!}Sn*p+>4cdkcA% z&7_+pMlceJ2OHa|a&1%D^j5nq^dlUXS-*{4>W(u(=p{+hNvmDNO&!q07mmTbJhxiplY=Ms`AEU-(>_xq7?=Ud9}4y30O?09@&5JkT@$hd8k!i{+5B~MWXsHGWocz+ zb$4E5$M_#*`<>sD-}sqW0E`T583j#1762>Te{m-HjkA@FrIC$+t*tJ==3hMT&!+x4 zU67@v`M-Pp?jmhL0Fd4P!>`@P<==npcXKo}w*-I;{_VhiJ^flh`~nRa3E>`2!4g-v;W%HU=YI3h=^!I(_3qr z*39OoipY4Fh%?_58l^0kU9g=two?3-Wg)aOX%|4!9aR3YMUhpHma13OxN3`zyPlyo zXNG$kQcd?Hk@Ub6oRjvV&H?y>OFPz=p5vkUP(>zUdBXcj8VIp z2%HF)2+3s{63SGnQ&?FiH4>e5kIVCqpgH=7=G{KkWP>amC$2~)+Oh%f>z|WzB7ZQX zLG5dtcnhbE^)_ct!j@Qo_<@B*SX>gUW|0NByDPM`z6nU~$(%V52SbWYu1fJv=$lVq zDACuoO;%qN!5j*HYV(kowzAJpa674EH>l7d3uU^=n_lQpr5C6+#gi$hWi!dv5+|-+zPjQ%(Mia_9DoQ4{~~ApNQ?f5!RyY_xuZ^HXj9 zZ&9LugYr{-{*3bX82A1LhvF+A8YMb>izp#dwqxVUzWX7 ps~DoClose(); } +void ScExportTest::testRelativeNamedExpressionsXLS() +{ + ScDocShellRef xDocSh = loadDoc("tdf113991_relativeNamedRanges.", FORMAT_ODS); + xDocSh->DoHardRecalc(); + ScDocShellRef xDocSh2 = saveAndReload(xDocSh.get(), FORMAT_XLS); + xDocSh->DoClose(); + xDocSh2->DoHardRecalc(); + ScDocument& rDoc = xDocSh2->GetDocument(); + + // Sheet2:F6 + ScAddress aPos(5,5,1); + CPPUNIT_ASSERT_EQUAL(18.0, rDoc.GetValue(aPos)); + ASSERT_FORMULA_EQUAL(rDoc, aPos, "SUM(test_conflict)", nullptr); + xDocSh2->DoClose(); +} + void ScExportTest::testSheetTextBoxHyperlinkXLSX() { ScDocShellRef xShell = loadDoc("textbox-hyperlink.", FORMAT_XLSX); diff --git a/sc/source/filter/excel/xename.cxx b/sc/source/filter/excel/xename.cxx index 2b6d991..98f9761 100644 --- a/sc/source/filter/excel/xename.cxx +++ b/sc/source/filter/excel/xename.cxx @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -524,6 +525,32 @@ sal_uInt16 XclExpNameManagerImpl::Append( const XclExpNameRef& xName ) return static_cast< sal_uInt16 >( maNameList.GetSize() ); // 1-based } +/** Returns true if defined name was not 3D. Fixes local defined names, but only reports for SCTAB_GLOBAL. */ +bool lcl_Ensure3DNamedRange( SCTAB nTab, const ScTokenArray* pScTokArr ) +{ + bool bFixRequired = false; + if ( !pScTokArr ) + return bFixRequired; + + formula::FormulaToken* pTok = pScTokArr->FirstToken(); + if ( pTok && pTok->GetType() == formula::svDoubleRef ) + { + ScComplexRefData* pRef = pTok->GetDoubleRef(); + if ( pRef && ( !pRef->Ref1.IsFlag3D() || !pRef->Ref2.IsFlag3D() ) ) + { + bFixRequired = true; + if ( nTab != SCTAB_GLOBAL ) + { + if ( pRef->Ref1.IsTabRel() ) + pRef->Ref1.SetAbsTab( nTab + pRef->Ref1.Tab() ); //XLS fix + if ( pRef->Ref2.IsTabRel() ) + pRef->Ref2.SetAbsTab( nTab + pRef->Ref2.Tab() ); + } + } + } + return bFixRequired; +} + sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRangeData ) { const OUString& rName = rRangeData.GetName(); @@ -544,6 +571,7 @@ sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRa This may cause recursive creation of other defined names. */ if( const ScTokenArray* pScTokArr = const_cast< ScRangeData& >( rRangeData ).GetCode() ) { + lcl_Ensure3DNamedRange( nTab, pScTokArr ); XclTokenArrayRef xTokArr = GetFormulaCompiler().CreateFormula( EXC_FMLATYPE_NAME, *pScTokArr ); xName->SetTokenArray( xTokArr ); -- 2.7.4 From 8a3d8732a02e00167b7fb0545bbce08b3a7f9402 Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Mon, 7 May 2018 16:51:22 +0300 Subject: [PATCH 4/5] tdf#113991 sc xlsx export: no relative sheet in named ranges The MS formats apparently require absolute sheet references in named ranges. So, local sheet references are now marked as 3D ranges during export. This patch only affects XLSX format. XLS was fixed in a previous commit. This bug only affect MS Excel, which complains of a damaged document, so no unit test included for this patch. LO had no problems roundtripping the problem document. Handling GLOBAL names will need to be handled separately. Change-Id: I2788824253af0d5d4f9ec4bbc03c5cdcc1c3b9b5 --- sc/source/filter/excel/xename.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sc/source/filter/excel/xename.cxx b/sc/source/filter/excel/xename.cxx index 98f9761..9165cd5 100644 --- a/sc/source/filter/excel/xename.cxx +++ b/sc/source/filter/excel/xename.cxx @@ -541,6 +541,8 @@ bool lcl_Ensure3DNamedRange( SCTAB nTab, const ScTokenArray* pScTokArr ) bFixRequired = true; if ( nTab != SCTAB_GLOBAL ) { + pRef->Ref1.SetFlag3D( true ); //XLSX fix + pRef->Ref2.SetFlag3D( true ); if ( pRef->Ref1.IsTabRel() ) pRef->Ref1.SetAbsTab( nTab + pRef->Ref1.Tab() ); //XLS fix if ( pRef->Ref2.IsTabRel() ) -- 2.7.4 From c20b17d0eda2c0813891b0465c252bc44a45ebf2 Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Mon, 7 May 2018 19:29:49 +0300 Subject: [PATCH 5/5] WIP tdf#133991 excel export: emulate relative GLOBAL named ranges The MS formats apparently require absolute sheet references in named ranges. So, it can't directly use LO's GLOBAL relative ranges. Instead, attempt to emulate that by duplicating the GLOBAL name directly into every sheet. If the name already exists, then (in theory) it would have overridden the global one anyway, so just drop it in that case. Easier said than done. This seems to work well for XLSX, which is the main concern since sheets were opening as corrupt before this. This is NOT working for XLS, which was silently displaying !REF$ errors prior to the patch anyway. Now the GLOBAL formula name is missing from the cell formula's, since it didn't find an index number in the lookup. That will be rather tricky to figure out, since the call is asking for the GLOBAL tab, and we need the tab that the cell formula is in. So, there is a tradeoff here. Do we fix the supported XLSX version and make the obsolete, currently non-functioning XLS version even worse, or do we leave status quo, and neither of them work. Change-Id: I32ab6f957a60fde7ec8a1912cfb0974a55db3886 --- sc/inc/rangenam.hxx | 1 + sc/source/filter/excel/xename.cxx | 32 +++++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/sc/inc/rangenam.hxx b/sc/inc/rangenam.hxx index ae3cefa..486e0f1 100644 --- a/sc/inc/rangenam.hxx +++ b/sc/inc/rangenam.hxx @@ -117,6 +117,7 @@ public: const OUString& GetName() const { return maNewName.isEmpty() ? aName : maNewName; } const OUString& GetUpperName() const { return aUpperName; } const ScAddress& GetPos() const { return aPos; } + ScDocument* GetDoc() { return pDoc; } // The index has to be unique. If index=0 a new index value is assigned. void SetIndex( sal_uInt16 nInd ) { nIndex = nInd; } sal_uInt16 GetIndex() const { return nIndex; } diff --git a/sc/source/filter/excel/xename.cxx b/sc/source/filter/excel/xename.cxx index 9165cd5..d548a6b 100644 --- a/sc/source/filter/excel/xename.cxx +++ b/sc/source/filter/excel/xename.cxx @@ -573,13 +573,18 @@ sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRa This may cause recursive creation of other defined names. */ if( const ScTokenArray* pScTokArr = const_cast< ScRangeData& >( rRangeData ).GetCode() ) { - lcl_Ensure3DNamedRange( nTab, pScTokArr ); - XclTokenArrayRef xTokArr = GetFormulaCompiler().CreateFormula( EXC_FMLATYPE_NAME, *pScTokArr ); + // Ensuring 3D for export: Modifying the actual document for emulated names would corrupt SCTAB_GLOBAL items, so use a copy + ScTokenArray* pTokenCopy = pScTokArr->Clone(); + const bool bEmulatedGlobalRelativeTable = lcl_Ensure3DNamedRange( nTab, pTokenCopy ) && nTab == SCTAB_GLOBAL; + XclTokenArrayRef xTokArr = GetFormulaCompiler().CreateFormula( EXC_FMLATYPE_NAME, *pTokenCopy ); xName->SetTokenArray( xTokArr ); OUString sSymbol; - rRangeData.GetSymbol( sSymbol, ((GetOutput() == EXC_OUTPUT_BINARY) ? + ScDocument* pDoc = const_cast(rRangeData).GetDoc(); + ScCompiler aComp(pDoc, rRangeData.GetPos(), *pTokenCopy, ((GetOutput() == EXC_OUTPUT_BINARY) ? formula::FormulaGrammar::GRAM_ENGLISH_XL_A1 : formula::FormulaGrammar::GRAM_OOXML)); + + aComp.CreateStringFromTokenArray( sSymbol ); xName->SetSymbol( sSymbol ); /* Try to replace by existing built-in name - complete token array is @@ -588,7 +593,7 @@ sal_uInt16 XclExpNameManagerImpl::CreateName( SCTAB nTab, const ScRangeData& rRa record for this name and all following records in the list must be deleted, otherwise they may contain wrong name list indexes. */ sal_uInt16 nBuiltInIdx = FindBuiltInNameIdx( rName, *xTokArr ); - if( nBuiltInIdx != 0 ) + if ( nBuiltInIdx != 0 || bEmulatedGlobalRelativeTable ) { // delete the new NAME records while( maNameList.GetSize() > nOldListSize ) @@ -668,13 +673,21 @@ void XclExpNameManagerImpl::CreateBuiltInNames() void XclExpNameManagerImpl::CreateUserNames() { + std::vector vEmulateAsLocalRange; const ScRangeName& rNamedRanges = GetNamedRanges(); ScRangeName::const_iterator itr = rNamedRanges.begin(), itrEnd = rNamedRanges.end(); for (; itr != itrEnd; ++itr) { // skip definitions of shared formulas if (!FindNamedExp(SCTAB_GLOBAL, itr->second->GetName())) - CreateName(SCTAB_GLOBAL, *itr->second); + { + // Ensuring 3D for export: safe to use here without a copy since no modifications occur when SCTAB_GLOBAL + // Unfortunately, there is no clone option for ScRangeData, which would have been easier. + if ( lcl_Ensure3DNamedRange (SCTAB_GLOBAL, itr->second->GetCode()) ) + vEmulateAsLocalRange.emplace_back(itr->second.get()); + else + CreateName(SCTAB_GLOBAL, *itr->second); + } } //look at sheets containing local range names ScRangeName::TabNameCopyMap rLocalNames; @@ -691,6 +704,15 @@ void XclExpNameManagerImpl::CreateUserNames() CreateName(tabIt->first, *itr->second); } } + + // emulate relative global variables with local ranges + // creating AFTER true local range names so that name conflicts will be discarded. + for ( SCTAB nTab = 0; nTab < GetDoc().GetTableCount(); ++nTab ) + { + for ( auto itr2 : vEmulateAsLocalRange ) + if (!FindNamedExp(nTab, itr2->GetName())) + CreateName(nTab, *itr2 ); + } } XclExpNameManager::XclExpNameManager( const XclExpRoot& rRoot ) : -- 2.7.4