From bb952370f75fbcd7f09f65bc6dffa97428422670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20Sz=C5=B1cs?= Date: Thu, 24 Sep 2026 12:00:32 +0200 Subject: [PATCH] tdf#169795 sw: fix crashes around tracked changes in cells with formula MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copying a cell over another one with change recording on could crash, or destroy the tracked change it had just recorded when the target cell had a number format. The root cause was that formulas and formatted numbers overwrite existing cells and destroy some tracked changes.. The cursor could point to a character that is not exist anymore. So, first i made some cursor position fix in some key points to avoid some crashes. Then disabled tracked changes when a cell with formula is changed. Every time it was recalculated, it generated a new tracked change. Normally if we want it to be tracked, we should track the formula inside, not the visible value .. but we don't have that type of tracked change. ChgTextToNum() could also generate lost tracked changes: If we copied a formatted number cell content to an other similar cell, (for example a currency like $1,000.00 to $10.00) by replacing the text ... then at first it generated a cell content like $1,000.00($10.00) where ($10.00) is a delete change.. Then the ChgTextToNum() reformatted the cell.. and overwritten it with the $1,000.00 ... so we lost the delete change. now i changed it so it can keep tracked deletion changes as well (it was able to store some insert changes) and then it checked if the format is fine for the real text .. If it is, then it will leave it.. so tracked changes survive. If the format is not good, then i keep the old way of working to replace cell with the new value... and make it 1 insert tracked change I do this to avoid complex problems.. because at format changes a lot of things can change.. position of the characters .. and it could be a confusion where the old delete change should start from. It would need more planning to figure out a way to handle it. Side note: in some cases if we modify the value of the formatted number cell to something that does not fit the format, it can lose the formation and change to pure text. (so all track changes will work on it) like if you change this: $1,123,234.00 -> $1,123234.00 (This is not my change. i just mentioned this behavior.) Signed-off-by: Attila Szűcs Change-Id: I152ec4720ec8e133800186c7e5787fe0f6954fc8 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/13456 Tested-by: Jenkins CPCI Reviewed-by: Stephan Bergmann (cherry picked from commit 699b9cb056cb283fc2699763e1a2340e12680c3f) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/211684 Tested-by: Jenkins Reviewed-by: Xisco Fauli --- sw/inc/crsrsh.hxx | 4 +- sw/qa/core/undo/data/tdf169795.odt | Bin 0 -> 13287 bytes sw/qa/core/undo/undo.cxx | 180 +++++++++++++++++++++++++++++ sw/source/core/crsr/crsrsh.cxx | 19 +++ sw/source/core/doc/tblcpy.cxx | 17 ++- sw/source/core/inc/UndoTable.hxx | 6 +- sw/source/core/table/swtable.cxx | 21 +++- sw/source/core/txtnode/txtedt.cxx | 6 +- sw/source/core/undo/untbl.cxx | 102 ++++++++++++++-- 9 files changed, 332 insertions(+), 23 deletions(-) create mode 100755 sw/qa/core/undo/data/tdf169795.odt diff --git a/sw/inc/crsrsh.hxx b/sw/inc/crsrsh.hxx index d21dac474ced..288bd88f202c 100644 --- a/sw/inc/crsrsh.hxx +++ b/sw/inc/crsrsh.hxx @@ -664,7 +664,7 @@ public: // select a table row, column or box (based on the current cursor) bool SelTableRow() { return SelTableRowOrCol( true ); } bool SelTableCol() { return SelTableRowOrCol( false ); } - bool SelTableBox(); + SW_DLLPUBLIC bool SelTableBox(); SW_DLLPUBLIC bool SelTable(); @@ -870,7 +870,7 @@ public: bool CheckTableBoxContent( const SwPosition* pPos = nullptr ); void SaveTableBoxContent( const SwPosition* pPos = nullptr ); void ClearTableBoxContent(); - bool EndAllTableBoxEdit(); + SW_DLLPUBLIC bool EndAllTableBoxEdit(); void SetSelTableCells( bool bFlag ) { m_bSelTableCells = bFlag; } bool IsSelTableCells() const { return m_bSelTableCells; } diff --git a/sw/qa/core/undo/data/tdf169795.odt b/sw/qa/core/undo/data/tdf169795.odt new file mode 100755 index 0000000000000000000000000000000000000000..c675f82c7ed58d8caf4cc4ea1c8e56dd17b152cc GIT binary patch literal 13287 zcmeHuWmH_twk`yh03mqrKyY`0CAhmwu%@xbT|;muxVyU(+#7dycXx+J_8mEUpL^eV zf8L+>u zD}bJrmAQ$5o}G!M1%rcyA-$!Zt%)tYr4_)!(9*yj1hBB9w*xrY$^Eq$JUskgi@oyy zi;!P?AUz8cBY>?Py^W#KP}GoxHzS(Q36C#ecvgS`2F8Z6zuq*hjm=LDhT%LBeYqtp zN<}`qa6fHwqf~@tC9FE>NDsd^xH7+8iPeCLykE??W{-xuk-k1>fqQtwOVZE%8q;flvggejht;{014xZkE^!d9L=1qxBx$<_Ygi z9^F%#u#ws20OZ^)oz>>@V=eC$w-{rWV*?laiSDzNkI}lx%*V3(jh~%gcAn`+UU1jT zsvU&P1%|VL_YphQ=S0e)Wpa{m@YOfQ<1mmA5Z+J_5dWpVKYQ^n-Mx0uUYnq$g^>x+ z-sZ1euw`cW&&o!8oo#F9Y!0xccLJFoX)akVvZH&pw(4h)-h7grM_p*d&O?Pnp^Mo3 zE_rVy3!|Fp;G8SumD#lXc$Ko|-%=C1Q7wFu{01iW;v?{OV=zK+%+k|s@OIs+KQi!f zIJoqy<(CUJGFtvc=T^77JkNut+ha>h;l#@LWAt~bxI;1D3R&{D-f-`14a1)_No;@q zBB63@6FFv+x1K_2e9V0`znh{0(DeCr5hj7zNZf`3YfZjMqUN{8&Dcu7jcLoq8@nj$ zOB7s89CYL?y7v)y=U#Gc$E}^Bq5-I0vVfwqA0;cb7ACJ3A-5Lb5(WU!w1vGojg)@Y zUz5~=B0t$%0N+~Ns9eeq9fiy~d{fUyqA5opqw`T>BZ^GkHvLL_tDvb)s@nH>T$C+1 z)~Uz-1=T+=Cp=teI9I~^Gf&jhqw@Hw!%0*BwbF1ZpS)rDC>niU@R?1dl>FR}9_%%y zHEU^U_0mZ-CAT}r@61HJc@7Rghf!VP+badvw(?hC@st)|*x8@yQ0+fS2**H4Ml=_D z+^fm;$qD^Bu004DwHLB!Fc!vu4Jc3q%LClF0qK)p>ThAqKt|@FqqWz>QHDyN%WDvc zKYEqjqK2e;3=1DXi4e^JJO=*UBjuWufPy^a=a0QH{F&a=)ef36eEedEv*-8Cs| zJUHNJDSbOVgmm!%XuIRC8-Y~)nU*gKf6yy=D;;-Z=kv))Hrf`y3+Kcx9k<}eW&Tlo z70ixio{GvtV#Hj1wH7$i1e9Xk786E8?g4h_VMdvtkWB{W(jfJ(sjJ29gWF~;^SiEl zym3KB3~TEjgL5c_3$b2E_~qG6R8epIXRr5I2KZF(0;EGXbMCzh+NzGWv-c0Bj3uZo zM#?M<%_hD?uJ`JX=p9+Meoq$5(YIRec8-<^azzwHqSqyQ(W_}$j;N_?3w>krX{Tn> zs9xFtK3VBdhB2A}G3P><-t7_87%dtZ5QRD&t8^FcsN+-(V9mK-*^btHTWB|}a4m85x?z#f7CcOoJXhz@N?3fZ$duKM5t;F3D)njGU0 zosB-N+|q#gDa0TQB&mi!<12_3xMHN0%x9GA2>xh|Snca7j*xt#ccq3*p&#oGWQ}MaYtK6^MG4Gg%tu#h zTV6c_5!pP{4;+0x-URmC&KNo%xjKURLdWqkQHlx&&h`gc!BD5wolEP8BG(|Z_yjvy zYdYS_mVJ%O3yL!JAsLl~DAWC|8+sQmD+V%78=Ylm$tW=#PJyQbC;Bkf8MA%cNV0O$ zdASq=&VhKmwqq)~W1bM)9X9uZvSFBF-doj}ttjG4$DN&#U~p!?fpS6yBkG$W0UoEZ z9IGQ(1sbi4!dW3j63sSK_P|ZH(q}yDDTg_5kBbMbiHO>r42nvKYLpci&WR98ks9z>`g2l?BK zsv)&;)YmzjSJ8r!4DJa&q$o-RXVZc^DHIrWn2GQY$Q7w*2~JV!lg{sp(ThLAy1c*c zJoCsTGucY&TT9PqIk@bR?qkZUhWwVkHUqdl&>`DNRbR!0SsvhtIcFgyA$Vg3lbSg# z=^kV{fAf{9*kbKFCSB4XwjkP%76ie!qEnuP2UAiI=3f~ zmE=i?GTax=d>e*hM+Jgzem%|y_?d)<=J1oCg*yw!<|Wg%*xVZY8J2Jo$A$VmL{E}s z!n5yYr*$vx*z(!fmKM>|lh)+Yz~IMY-Z8doM>_&tiAZ3sc~acB$jU1?yo(Y6qMCtH zp@-OqXf&C;v%si)lTJb_3IEE}?g{6@6DU6x8iKt?;9YN1m9ZIO&7zEcZPXJo=F5ho z9aC;kE{pOt4EWkM(#`A=YExx-AM>A&W7 z)x4Otv0UG1T>z0&v?zV(VB;I$tCYi1>++-Pz3_MM<9<5v66c{tYyQ$h`9O^QD-!ea z-pTm%YyuxLXj$m$2(Dktws{!O-0uijS}PVU71xfY>N1mc>=JYMrT7W5^)U49g7<8N zy#b}^&-_u7sHagK8@DMsgGtX z($m{9#f0iJ6NcKqZyXpU7AO6lu_O~^|L~jTh*f<-wesG7{IsKh_0co4h{n`59!Xe| zQtUjMfZ7%28$?DpHRXVR)Ac1I@z+q_j8{4P5ldJC=a8uj9em2h|*0V59dxHKP#yQI61|+*!plQ4R=0m8vGPqhR=6o$R?+VT{YMN*wxIW-R$d6hU7=Z6 zUa*|c;slA{F;w#qQM)A?kPTI-F4ScEeq3>qal#a-8xmUbCXat}8wmdR2{G#(viRBC zQJn0#tCJ=LE0u2&k$B=(fnRsrFC_}}*(u`Iy_^fgDVUjoS1Qq?Iyu)K#f@av?v7+5@y8K5-^p(&) z)q;qU+nKmT(l3=-YfH-{Z~s)=gE<>nGsb6shZ7H|-}sW~>~#1Z0RkeQ;}5?4Pw?Cp zU}tAy0sIRyHfw4+;&CB+tb7~mv>jihls1Xt75FM4`36qR)?sBZ1i~_ex98gD^hEU> znRS3%8m?NF%L}(JGDkMe@M3FZr@-X~dRjb-L4x8WT=GOf^i!;hn|uQiE9!`(r_S?P zcZdk{4^#6@JH&Y+?o&b{>yuOaqr$lv?3~H{!gn$ag;ZZY>|ETm`1+r}@jMl0duZ|TCk-u+40lvfr180PBd*XBhK3OK zooQmn@}|(ye%nNviI$8*;-?*KeT;04eBz2G%-u|Kyz9w6%=DR7P9qzP)u92ooV1=i zw(XcRXWe* z#Z+yec4yyE7)^WQWWnOS&*)iIVMPv=C$`%gU%&|3Gu~@pV}Yytz zDYuW4N}}U9y$ykMqFcUjx1hBgq#C>56tig_v^BA=S5 zU7>0ecbrFM(PaZuxa+{aeJiW*X6RDW^~I0xGdAvWXwCsiSp2$buRu1NqKOr`k_hc; zu=vFH8&W)eYX6pZI1nyxD|`ozq6fG&LXn1wg2IPAp)&D~`@=dU&jX5$*H7kvZwBv% zaHR&IZ^q!e@M%S*vz`WLMPZfSX0gc`FMZxM`+0dv1j|-ZA#yTR;lMvFLU;%&Y|55c zh)!1-7~sRm%MhKyydDNtFenMg1H3Uo&s=}uI+UaIQYNQo*p$ZJrLJKOW!-f z8_wW`C(W}uV3yyk?a=4q9q%l0du*yZU7F#}OptUjb*Xj)XIPa-m5Z3tS@eI!dOAv) z6zL|{-{o1RU|Ju9bz3RnoLc`yQ;Y*&T<|0$^eZz_RIEx(s2;9`9$I#IOZe^Tz{f# zKO~;fSPL?wvMBHF z%Cjh{g4gXek@XTvcpkmdGPL-!Hmc=7XiP|7>g`d%U?nq)oKMBW;Ib=cMQhepoOuFF z1}bWAKbif4EA!6x`KZ&GzvP`9Ey}OQtL&4)P(s&=@L8+9s+g z_78qV^knJu8-2$(IYYkrQ@K@lqtW(RtGx~Sd<7y0M+VA9n*n)!s?Lr?x`MD+44sf@ zJCE{i6B6?Nb1_M`Wi8^{tdtPX(x$A6bG&w=;~#Ca^Kf@XuiMQE8P243XQH4!ic!*CnsO{OCw$CL=`tneG=u#Z+ab^nZPCs z6iDEZCkLG1WLbaB65iunZSvP^4YWveY5n7V^qBOovW zHA&2?VcGLnGM|<0QxU3vk;eJXT-3hVi09`Gn+>t*6v?P2h*q$gFqO))VChRBiBcIo zup`**1+!MQv40pT>@a#@eA~@7G(Xw2THk4AJtyz`nop-aFi zn@{@IYt4M2f?rbSGW97i$Lq?*rcKUyJcDA}PilznmnPj~50m51Fz(`EVF4)2zlakh4RH&Sqjaxry!yw`4Tj{nh2>-^DfWvX zeJqcEQeB!R9Emh$QGrbhc!LG*03)Rve6N`@7(^Cxj@Uj13a3BxOABVraEq3?`%Yz$ zL~`H*PFN!zNs7smQLw=$`%m;Ud+6-H#y(=VqrfDqy&1ZJGjiT4-ewBq&H|Xce5>z# zsSh&8xJd)pRmt;KAUBkDD!zv~K2$yI3|uwZl~n^Xmp8{W0FCC>R3$~Jli12pfEL9e zcYXqnpIG}31V41a`h07=FnrL2*XU~-PzH= zzRy$?nFG?sOp4fLcyjG^LPL7+aYbajMOim# zl3DbY^ngP8&W7)A_Qh@2DXxm@6S)$wL2XD^9|QFb3|nhr{*ke;3R5o@JyiMmFP|Mg z*IHB`BlDlp3@seSwk54_msujKzdX|VDAabOwsW19_ez~oD+h1BddIePBi`#DSi*%( zd-%FkAKlc)y}bDiUWV2C`n_7+r$iD2W#ac zK+MMRb&nKk<@%(9z-jPX&i0$YL0=I>=YZpS6t)bT)(Np+{6@sG(lc& zq>Ds`Wa4$GNOwGPa&i%+k9LQE5Ev_h7^8EIXKW;Ayw#`$w2-fTMJp2teLaSN>9$=6 zj^!oijLtcHoJ14Fkum2Oc?lb>7=T2i5po`htZV(zLSakWL^;<2+s^r{zsA&?cjVxE zQO!_RZ4(qjQWwc(aB*{JY<{YDn%dDg7%`XTb~IWS!}Z0GKp&m7vd0Ik86;l4r2FH& zlC_ead+Xl91k(r-^RSzdQDr?}SqBsb$UfJ>Mf^OsS zRkb8ThvuO3k)V|$Rkg`SDI zErZ>kF1?inFi=iL^erOZZ>Q;7aWNqU2nfg!2#7Zj@UI?Oh+Ov~k>9*>QcA*1OiVmH zJVHW3l9G~&ii%oVS_TFN=H}+MwzjUWuHN3>!NI{%QBg@rNm*H0rKP1+RaH$*O`V;c z!^6W96BEnJ%NrXThlhukmzU4a&#$Y#yu7^KAEkNiAy|p2+d@Dfz5m@HArg~c1xCve z7y6>)G=G#5s6Y7&T^Oh{-&k|<@{%QM1NR0BN~hA!&-Q)#=dR<*Fp0|k$o;Y(gd(2u z&}{;35U_7x8POQOdwqvQhDO#Ue!U=|Afa&JasF-aU&H?x1T#b{J$y@$$DIHBsk0p(JMMEC6Dx)V}|?UR^A zh!l`)d3|;B#8I%nI-x?@>-z<3pe|jV0ta^OESI&`Zs6SI;_WsmVccAY#hjVCiB;H~ z#`;^hzhr+lWBj3TKVH}5@GEkEAAxTk;>MKGt|dTAo9B7sE#-+`+qdZMjp#Od9M};j z%8~NzEiawKk$CipoJz*pF4vq{usErgAs-2)V+Jp|<}!X}K!;_m)(3s>RNTl@Y z#;yz6Op?=eT1IU9Ok6e7`|k6t^M1$)|Y&97A*Rw-h}+pX(UBg?{Y&bdzE%As3g%lrdI%bi3Z9eNZb>?^TZ!uRRyc47bP=uJes1@6lB;ZY# zvFV@M_N8rGNEFxb*n!>)sc%M$@aCv@O&zjz2cx@wewdQbPAZ_CnG3fLs#@5?1?UKs z!b!DwxKKJ56Y(~3l2>T(7p=qzJy1}W{2BjoOEOo`%IhIqzZlCEwme4jh%rGAOs|MhOC zhInoo`?wrZJogA%52@{zyU=Q{#?1S4zDbk?^VuVi)c1-LnHPhihVKYBN{Nf7MnfLX z!vy8x3)FhCvb$pXr|f#jc>)f!R0`nk4rydWy)gqN7sYh?fr<& zeu^=axMJf(-4$gQ*y%*m7wfBU>ERgAE}4wA_>?O{A_B z%MTHJMN+uGDhK}3_m2cu-gDLotMwxT5JY)8Wxy7tteZ^3HIu*+cMu&1PJMOUc#9}Y zOwp2QND6by+P869RCYxd6QvsYq{-bhslB@1a8vJ6pK2)zrEdzlN~I^DLsEPzc6FWC-BW9&Q^+AG zf08V$TB&4~1h{0$Id<0*Q}^KD*qgEDPO4=0;61|iKAWp07AyUD`fKy-G<~E&L3F8~ zyP}KRqPVk%D-7_hyd1^VJwE6mZg&2(WhgrNQqAld{~jQ2)0?s%gyF)EoUx9B3d__| z^7caUgYMMEl7dSCN>A?iM7#!Pc6aVQor64sz!d=HvzcH4<30r{0Nq8dPRT~KVY079 zs$rx`XJw20tc*@kOWf7>Fcp8p6}*xtc4cWtDd642s~fZ+kxc|z;; zR)lx?txH1I>Cq57+;9ot7KC$GlSEZ>O(&uPCJ%Wl67Rr2lV2i4|Kw{_=%AtUVNX$> zB8e*Ma8}pNBv-20$HbFmipOA4x7srnh_j$N`@Y_7%18o1nL$i~Aq&3foOGI@+uzPS?`VbX0fNVVaCr2u|{AV2ri}v!UsM+&V{yd@Gdr_vGf`0!%t?nrx*RsWq%&SM}G!cv5OQ>9I>&HVKnHam#O{iV03uv=rr7;N0rW z-Q5+Xkdbv8222-4nHaz82d!V&OhOcW!p^CXzy+Ynx_P{wHVZy(M4*Z}@J6nos_UYa zi+G#6NNZie^@q6>4fIuj_|Unj&|o)#7J8cxG#D3^YnR4T2sFEc%QxLPkF;WD%#4aH4NYQb zOmEN;H(@%dwk~qll*ofYS0dd%yEEtKO`-JQ~JCWm?iSz60o*WQRG|fn&BR0?ErZ+IO3e$LV=?>1;_a7obvnsn!;O}zx`12R>RIrUX z=?*O|B{MAMB2V}42EEDWU-7&z!9={@VqI~Mh4{5VruK=b>ZggKF=;(}x-p9ri--3| z%T(;impPfTljlQ z)7xAyH3y;htd5GBh^DGc$Q4N1$BV0Aw1@JAXqU&Aa{@31Hpi@ocZFruqQ(ce6H|ck z1R;1bZwF&??^(#>4rChjR%D(ht1hZ3FCEKlPrH@3{zTzqBJT{@OtUbp@(Oo&tCqNl zz1f-r*;0~u*pJ)ovK#CYi6&KD12yYvmL&DG>k?3wB27IvJ5JcVFd3Wk;?Soh`30nP zbWA;U9k5)Jpv@Jx>CQ;yk`_6XmOFAg0q1W1d?n!B0(Xvl7miH0a(q+wdG8d}SpbEM z&Mjr1g;dAwOsHyDNF!#Ny1RfG>2Dg~_3 zT)A{tKUCb+mvBdV>)3UW=Z#1}80u3bXYsCjQ4QF^6YQoB?!`OydcBXA$yc0j^~N-c zIvM>g)WdBN>W+1u%oiBXg_z&p-OMu&T&@`Ls`&G4aOt4myEdnF%9)AQ34alGU#U~V zn=;V1Z{3;1vz(OjRbG0OMBKM$J9Fp#7{7B!H^g#y_V%M-#nTeYy4^6#p<$@|b%==2 zY^<_oZIn6Vd|W9Tc#5ezis^1Bv4F?7@;XDYBV1vR3E97jaHS)M&meVXGfk>wBDmDu zZxT*VjVV|w6n-jTDtVvfk;7dCgrB>ZP{5@wxM8`6WuY=$qXxUyrLXT!v~p^(zfnn8 zht%T2VZQ~<-8PJ>+K0UhFx58SM%6(?}?C0 zX(N7~Ymzzj3U+rHy&M9M_bf(5f#Y7=i!M(174qV&T_1*`Xy93g&MsYy@ocJMU5TX3 zq3<%gqNVpPT8k8=xogJ4;~bB{z!6^BIE>pi)sKud^jGXnewEZ z`GFkhgZr@c;KXozoA(f9bp43R0Am?-ADgTTwM4@cK;F}_A(B69WxL?&MyT=LDV>q0 zZ@h!VKuT~$Mqz|_P%iY|CzYwNkD?L4OMjet#o2o$K(p3!CXvoKBAcP+F8jruv`Px0 z?wRZ0Y!uWvQ#xJ#4E$ik>y}0WsJMuD{W~a1s!b*}m`(f9Fx`5rFiZ{%F|A7ZIv-_t zD+;AmAs9Sg$~UOU%p8kVUz{4rmjH}sN7AM!8BA5dBYc@VBk_m$mp0bMU_m$$y%Xe-Hj| z?9AVT|9|0sDe?bS73uJQoACcn>fH_TiKtcwfG1opv-j%7fDjj!5i0$v`{O^SrXjt~ zGO)C;`<*fPS8kf7lY?3zgMJ@N*vCk+0Df5^V2#T(wp_QijgMPjtW*@EXmsys6Dk1mi;aWLcV_d0!X*~zB#D^-l$5((wWEe4oP?_%@m__{TH$u@&( z*|UsFB_!P#77%Hc3u$EyJ5iA7!rp@cCa7VtAT}x;tK357G}OoIbtunHq2TfwFRZoH zlAG;-I`$Z&<#Lw9nicXPt(dS0V?XDEt&f2Y?06l3IUEK_8M19mhq9kbf|E1}-q(7_ z@r%rfZ@?erv1QUbYM9UcV3NE&3{7%vdLY*4o1N8+GQa0A;UHA1qF}91#J?B*suUc` zVKjRwSZyw}HXFN6HNzSAZYhlkqeeI95YqUfOD!QIh@ChdOHc7lNs1p63;HK7#ojebQtbL{jHyK&l-lKOPBx2dDl-biod{C1E5u?T< z{fHas4?xo=J|H{xsKgjjqFUDQuVJ3ch@jaww^;43SnY_%0)97l^-)HFw!&hB+SHrq zA;g@RQSYGkcb%)6FAY3P6}X;wH`*x76pvGB*N*qDArXf`uA{4nuEl(G7bI13JA~F# z{t~g;6$xL_{u6D_E1Ut1f;&#_D5E|43ypbD1q|mfDMlJJ{Z&9X%B=a&#JtYY6u0)C zHbP*QqWPJZQ!)Zp?=r(KfBa#0P4^AitVa|17zl4hS3^-|%JK-Q$qhdTeNh^HWbz{V zUBQb*db6S;cTNkwiHb84)sx|;2q~sG=-^~$aW`dwm^U%oFjkSlgl?=B;ajV!SjmFwWR~P z$`4LNE{SazNZ8446*hJY@*wxFry8%*=HkR$H&xIe%o$|XQPX9ao1qeA4ykQ$9`7vy zss)se6H~J z8gLDTnRiK?uJu!&#qSR1<7_?jY?5`!m@xd;u<)X)wqj9?zvHoT9z0lM8sut`3&6-K zCXMUD&crWUZRhE98%@_P=Y2i@q;PkH{M{`i@Nmg0f)|R>QHpeaLZ2YR`9^@X5stSA zs}DRHMnQBie~f?vlao_L#{=na;0V3mKqw@SFR9OzE`w;vA*^&}A@Z=ex3B{<%NH~Jt;WgR;o9Y zaT@9{ZK_DjtnR;9~kvi;d<*T$KJ=LXMWjGaOOIqkY zpGCwYd$EP$TJ`#n$s~IWrO@U%KwR^jS2|v?x9O<5TunfsEqu{6Ou?48t=bb}5NTOj zWpMhs$-}_<#LHi)MQJhlPxWxGdHsBULNEWj)FRB+6JJ_b=?k5>j0nTON&sHZ{*owZ z(@sV--y1I9v060=No?V+zRc10*>G}4KUs)FeLTL}JoJB((UKtcJG^9ZS#kYnQuSF~ zk$^LvUtnX4h4gn6<&2lUE`L;biKD%EXy{7xa!H;3YyD+>4 zyU70vc0s!cH5z}D^lSHD+x1$p@rUzIcMuh)e-osaD`Og%#|IYc( zK;iF*@!wSNs>Iham%rl2f4Kg9%l{rk{F`eK>pzDS|E%&y>VLi|zY*}?q)hp5c=*3F k{&T~Bzesh$OV1G|Tc)f7mKtR+p{r>d70CGcOumAu6 literal 0 HcmV?d00001 diff --git a/sw/qa/core/undo/undo.cxx b/sw/qa/core/undo/undo.cxx index 339fed31c1f2..5355eb4e9249 100644 --- a/sw/qa/core/undo/undo.cxx +++ b/sw/qa/core/undo/undo.cxx @@ -25,6 +25,17 @@ #include #include #include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include /// Covers sw/source/core/undo/ fixes. class SwCoreUndoTest : public SwModelTestBase @@ -105,6 +116,175 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTableCopyRedline) pWrtShell->Undo(); } +namespace +{ +/// Put the view cursor into the given cell of the given table. +void lcl_GotoCell(const uno::Reference& xComponent, SwXTextDocument* pTextDoc, + const OUString& rTable, const OUString& rCell) +{ + uno::Reference xSupplier(xComponent, uno::UNO_QUERY_THROW); + uno::Reference xTable(xSupplier->getTextTables()->getByName(rTable), + uno::UNO_QUERY_THROW); + uno::Reference xCell(xTable->getCellByName(rCell), uno::UNO_QUERY_THROW); + uno::Reference xCursorSupplier(pTextDoc->getCurrentController(), + uno::UNO_QUERY_THROW); + xCursorSupplier->getViewCursor()->gotoRange(xCell->getStart(), /*bExpand=*/false); +} + +/// The text of the given cell of the given table. +OUString lcl_GetCellText(const uno::Reference& xComponent, const OUString& rTable, + const OUString& rCell) +{ + uno::Reference xSupplier(xComponent, uno::UNO_QUERY_THROW); + uno::Reference xTable(xSupplier->getTextTables()->getByName(rTable), + uno::UNO_QUERY_THROW); + uno::Reference xCell(xTable->getCellByName(rCell), uno::UNO_QUERY_THROW); + return xCell->getString(); +} + +/// The formula of the given cell of the given table, empty when it holds none. +OUString lcl_GetCellFormula(const uno::Reference& xComponent, + const OUString& rTable, const OUString& rCell) +{ + uno::Reference xSupplier(xComponent, uno::UNO_QUERY_THROW); + uno::Reference xTable(xSupplier->getTextTables()->getByName(rTable), + uno::UNO_QUERY_THROW); + uno::Reference xCell(xTable->getCellByName(rCell)); + CPPUNIT_ASSERT(xCell.is()); + return xCell->getFormula(); +} + +/// The tracked changes of the document, as a ":" list. +OUString lcl_GetRedlines(SwDoc* pDoc) +{ + OUString aRet; + const SwRedlineTable& rTable = pDoc->getIDocumentRedlineAccess().GetRedlineTable(); + for (SwRedlineTable::size_type i = 0; i < rTable.size(); ++i) + { + if (!aRet.isEmpty()) + aRet += " "; + aRet += SwRedlineTypeToOUString(rTable[i]->GetType()) + ":" + rTable[i]->GetText(); + } + return aRet; +} + +/// Every cursor position has to stay inside the text of the node it points at. +void lcl_AssertCursorInText(SwWrtShell* pWrtShell) +{ + for (const SwPaM& rPaM : pWrtShell->GetCursor()->GetRingContainer()) + { + for (const SwPosition* pPos : { rPaM.GetPoint(), rPaM.GetMark() }) + { + const SwContentNode* pContentNode = pPos->GetNode().GetContentNode(); + if (!pContentNode) + continue; + CPPUNIT_ASSERT_LESSEQUAL(pContentNode->Len(), pPos->GetContentIndex()); + } + } +} +} + +CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTdf169795CopyCellWithFormula) +{ + // Given a document with two tables and change recording enabled, where the cell to copy + // holds a formula and the cell to copy it over does not: + createSwDoc("tdf169795.odt"); + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + SwXTextDocument* pTextDoc = getSwTextDoc(); + SwDoc* pDoc = pWrtShell->GetDoc(); + pDoc->getIDocumentRedlineAccess().SetRedlineFlags(RedlineFlags::On | RedlineFlags::ShowInsert + | RedlineFlags::ShowDelete); + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetCellFormula(mxComponent, u"Table1"_ustr, u"D2"_ustr)); + + // When copying that whole cell, "$100.00", over "$10.00": + lcl_GotoCell(mxComponent, pTextDoc, u"Table2"_ustr, u"D3"_ustr); + pWrtShell->SelTableBox(); + rtl::Reference pTransfer = new SwTransferable(*pWrtShell); + pTransfer->Copy(); + TransferableDataHelper aHelper(pTransfer); + lcl_GotoCell(mxComponent, pTextDoc, u"Table1"_ustr, u"D2"_ustr); + pWrtShell->SelTableBox(); + SwTransferable::Paste(*pWrtShell, aHelper); + + // Then no change is recorded over the cell: what it shows is worked out from the formula in + // its new place, not text that somebody wrote, so there is nothing to accept or reject. + // Without the accompanying fix in place, the copy was recorded as a change over text that + // the recalculation then rewrote, and the records left over that text described text that + // was no longer there. + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetRedlines(pDoc)); + CPPUNIT_ASSERT(!lcl_GetCellFormula(mxComponent, u"Table1"_ustr, u"D2"_ustr).isEmpty()); + CPPUNIT_ASSERT_EQUAL(u"$10.00"_ustr, lcl_GetCellText(mxComponent, u"Table1"_ustr, u"D2"_ustr)); + + // And undo takes the formula away again, leaving the old value behind. + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetRedlines(pDoc)); + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetCellFormula(mxComponent, u"Table1"_ustr, u"D2"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"$10.00"_ustr, lcl_GetCellText(mxComponent, u"Table1"_ustr, u"D2"_ustr)); + + // The cursor has to stay inside the text it points at. Without the fix, undo left it on the + // range of the pasted text, which the undo removed again, so it pointed past the end of the + // cell text: content index 11 of a 6 character long node. + lcl_AssertCursorInText(pWrtShell); + + // ... and counting words over that selection must not run out of the text either; this is + // what the status bar does, and what crashed in SwScanner. + SwDocStat aDocStat; + pWrtShell->CountWords(aDocStat); + + // Redo puts the formula back, again without recording a change, and again with a cursor that + // describes something. Without the fix, redo went down the route the copy had not taken. + pWrtShell->Redo(); + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetRedlines(pDoc)); + CPPUNIT_ASSERT(!lcl_GetCellFormula(mxComponent, u"Table1"_ustr, u"D2"_ustr).isEmpty()); + lcl_AssertCursorInText(pWrtShell); + pWrtShell->CountWords(aDocStat); +} + +CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTdf169795NumFormatCellPasteKeepsTrackedChanges) +{ + // Given a document with number cells and change recording enabled: + createSwDoc("tdf169795.odt"); + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + SwXTextDocument* pTextDoc = getSwTextDoc(); + SwDoc* pDoc = pWrtShell->GetDoc(); + pDoc->getIDocumentRedlineAccess().SetRedlineFlags(RedlineFlags::On | RedlineFlags::ShowInsert + | RedlineFlags::ShowDelete); + + // When copying one currency cell over another one: + lcl_GotoCell(mxComponent, pTextDoc, u"Table1"_ustr, u"C3"_ustr); + pWrtShell->SelTableBox(); + rtl::Reference pTransfer = new SwTransferable(*pWrtShell); + pTransfer->Copy(); + TransferableDataHelper aHelper(pTransfer); + lcl_GotoCell(mxComponent, pTextDoc, u"Table1"_ustr, u"C2"_ustr); + pWrtShell->SelTableBox(); + SwTransferable::Paste(*pWrtShell, aHelper); + + // Then the old value has to be kept as a deletion. + // Without the accompanying fix in place, the number format of the box regenerated the text + // of the cell right after the copy, which dropped the deletion: only the insertion was left, + // and the old value was gone for good. + OUString aExpected(u"Insert:$2.00 Delete:$1.00"_ustr); + CPPUNIT_ASSERT_EQUAL(aExpected, lcl_GetRedlines(pDoc)); + + // Leaving the cell must not drop it either, that is where the cell is checked against its + // number format. + pWrtShell->EndAllTableBoxEdit(); + CPPUNIT_ASSERT_EQUAL(aExpected, lcl_GetRedlines(pDoc)); + + // And undo and redo have to be symmetric: undo leaves the old value alone, + pWrtShell->Undo(); + CPPUNIT_ASSERT_EQUAL(OUString(), lcl_GetRedlines(pDoc)); + CPPUNIT_ASSERT_EQUAL(u"$1.00"_ustr, lcl_GetCellText(mxComponent, u"Table1"_ustr, u"C2"_ustr)); + + // ... and redo puts the very same pair of changes back. Without the fix, redo produced two + // empty redlines instead, which piled up over further undo and redo rounds. + pWrtShell->Redo(); + CPPUNIT_ASSERT_EQUAL(aExpected, lcl_GetRedlines(pDoc)); +} + CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testImagePropsCreateUndoAndModifyDoc) { createSwDoc("image-as-character.odt"); diff --git a/sw/source/core/crsr/crsrsh.cxx b/sw/source/core/crsr/crsrsh.cxx index 119e8ab3421a..2cd9507cbd67 100644 --- a/sw/source/core/crsr/crsrsh.cxx +++ b/sw/source/core/crsr/crsrsh.cxx @@ -4276,6 +4276,25 @@ void SwCursorShell::ClearUpCursors() pStartCursor->DeleteMark(); bChanged = true; } + + // sw_PosOk only asks whether the position is in a content node, so a + // content index pointing past the end of the text survives it. + // It should not happen, but still happened when an undo left it behind when + // it rebuilt the text it had selected. + // Keep the offsets inside the node, so that users of the selection cannot run + // out of bounds. + for (SwPaM& rPaM : pStartCursor->GetRingContainer()) + { + for (SwPosition* pPos : { rPaM.GetPoint(), rPaM.GetMark() }) + { + const SwContentNode* pContentNode = pPos->GetNode().GetContentNode(); + if (pContentNode && pPos->GetContentIndex() > pContentNode->Len()) + { + pPos->SetContent(pContentNode->Len()); + bChanged = true; + } + } + } if (pStartCursor->GetPoint()->GetNode().IsTableNode()) { // tdf#106959: When cursor points to start of a table, the proper content diff --git a/sw/source/core/doc/tblcpy.cxx b/sw/source/core/doc/tblcpy.cxx index 986da9801782..3ee04912120e 100644 --- a/sw/source/core/doc/tblcpy.cxx +++ b/sw/source/core/doc/tblcpy.cxx @@ -505,10 +505,19 @@ static void lcl_CpyBox( const SwTable& rCpyTable, const SwTableBox* pCpyBox, pDstBox->GetSttNd()->EndOfSectionIndex() - pDstBox->GetSttIdx() ); - if( pUndo ) - pUndo->AddBoxBefore( *pDstBox, bDelContent ); + // Do not record (track) changes if the cell holds a formula. + // The text that arrives with the copy will be recalculated based on the formula, + // and that would eliminate the tracked change from the cell anyway. + // TODO: if we want tracked changes to work on it, we should create a new type of + // tracked change that track the formula changes, not the cell values changes. + const bool bTrackChanges = !pCpyBox + || SfxItemState::SET + != pCpyBox->GetFrameFormat()->GetItemState( RES_BOXATR_FORMULA, false ); - bool bUndoRedline = pUndo && rDoc.getIDocumentRedlineAccess().IsRedlineOn(); + if( pUndo ) + pUndo->AddBoxBefore( *pDstBox, bDelContent, bTrackChanges ); + + bool bUndoRedline = pUndo && bTrackChanges && rDoc.getIDocumentRedlineAccess().IsRedlineOn(); ::sw::UndoGuard const undoGuard(rDoc.GetIDocumentUndoRedo()); SwNodeIndex aSavePos( aInsIdx, -1 ); @@ -574,7 +583,7 @@ static void lcl_CpyBox( const SwTable& rCpyTable, const SwTableBox* pCpyBox, //b6341295: Table copy redlining will be managed by AddBoxAfter() if( pUndo ) - pUndo->AddBoxAfter( *pDstBox, aInsIdx, bDelContent ); + pUndo->AddBoxAfter( *pDstBox, aInsIdx, bDelContent, bTrackChanges ); // heading SwTextNode *const pTextNd = aSavePos.GetNode().GetTextNode(); diff --git a/sw/source/core/inc/UndoTable.hxx b/sw/source/core/inc/UndoTable.hxx index 39e2adbcd01c..a62569036354 100644 --- a/sw/source/core/inc/UndoTable.hxx +++ b/sw/source/core/inc/UndoTable.hxx @@ -292,9 +292,11 @@ public: virtual void UndoImpl( ::sw::UndoRedoContext & ) override; virtual void RedoImpl( ::sw::UndoRedoContext & ) override; - void AddBoxBefore( const SwTableBox& rBox, bool bDelContent ); + /// bTrackChanges is false for a box that is copied without recording it as a change, even + /// though changes are being recorded: see lcl_CpyBox(). + void AddBoxBefore( const SwTableBox& rBox, bool bDelContent, bool bTrackChanges ); void AddBoxAfter( const SwTableBox& rBox, const SwNodeIndex& rIdx, - bool bDelContent ); + bool bDelContent, bool bTrackChanges ); bool IsEmpty() const; bool InsertRow( SwTable& rTable, const SwSelBoxes& rBoxes, sal_uInt16 nCnt ); diff --git a/sw/source/core/table/swtable.cxx b/sw/source/core/table/swtable.cxx index 5eb4018e007e..c5ad983470e6 100644 --- a/sw/source/core/table/swtable.cxx +++ b/sw/source/core/table/swtable.cxx @@ -2431,7 +2431,16 @@ void ChgTextToNum( SwTableBox& rBox, const OUString& rText, const Color* pCol, } rBox.SetSaveNumFormatColor( pCol ? *pCol : std::optional() ); - if( pTNd->GetText() != rText ) + // What the cell shows is its text without what a tracked deletion took out of it, which is + // also the text HasNumContent() and IsNumberChanged() work the value out from. Ask whether + // that already says what the format wants: if it does there is nothing to do, and the + // changes recorded in the cell are left alone. + // + // If it does not, the text is rewritten as a whole below, which loses the changes recorded + // over it. That is coarse, but a number format is free to rewrite the text into anything - + // a number can come out as words, or grow a thousands separator that shifts every position + // after it - so there is no general way to carry a set of changes across a reformat. + if( pTNd->GetRedlineText() != rText ) { // Exchange text. Bugfix to keep Tabs (front and back!) and annotations (inword comment anchors) const OUString& rOrig = pTNd->GetText(); @@ -2475,12 +2484,18 @@ void ChgTextToNum( SwTableBox& rBox, const OUString& rText, const Color* pCol, while( nCommentPos > aIdx.GetIndex() && nCommentPos == nEndPos ); } + const sal_Int32 nInsStt = aIdx.GetIndex(); pTNd->EraseText( aIdx, n, SwInsertFlags::EMPTYEXPAND ); pTNd->InsertText( rText, aIdx, SwInsertFlags::EMPTYEXPAND ); - if( rDoc.getIDocumentRedlineAccess().IsRedlineOn() ) + // The text of a box with a formula is worked out from the formula and is rewritten + // whenever the table is recalculated, by a change to some other box as much as by one + // here. Nobody wrote it, so do not record it as an insertion. + const bool bFormulaBox = SfxItemState::SET + == rBox.GetFrameFormat()->GetItemState( RES_BOXATR_FORMULA, false ); + if( rDoc.getIDocumentRedlineAccess().IsRedlineOn() && !bFormulaBox ) { - SwPaM aTemp(*pTNd, 0, *pTNd, rText.getLength()); + SwPaM aTemp(*pTNd, nInsStt, *pTNd, nInsStt + rText.getLength()); rDoc.getIDocumentRedlineAccess().AppendRedline(new SwRangeRedline(RedlineType::Insert, aTemp), true); } } diff --git a/sw/source/core/txtnode/txtedt.cxx b/sw/source/core/txtnode/txtedt.cxx index 9bcdfddc2131..46953d1aaa5e 100644 --- a/sw/source/core/txtnode/txtedt.cxx +++ b/sw/source/core/txtnode/txtedt.cxx @@ -18,6 +18,7 @@ */ #include +#include #include #include @@ -751,7 +752,10 @@ SwScanner::SwScanner(std::function aGe , m_bClip(bClp) { m_nStartPos = m_nBegin = nStart; - m_nEndPos = nEnd; + // The range comes from a selection, which can be stale - e.g. an undo that + // rebuilt the text leaves the cursor describing the old, longer text. Keep + // the scan inside the string; + m_nEndPos = std::min(nEnd, m_aPreDashReplacementText.getLength()); //MSWord f.e has special emdash and endash behaviour in that they break //words for the purposes of word counting, while a hyphen etc. doesn't. diff --git a/sw/source/core/undo/untbl.cxx b/sw/source/core/undo/untbl.cxx index 3307b43b07bd..8e9f03e49b5c 100644 --- a/sw/source/core/undo/untbl.cxx +++ b/sw/source/core/undo/untbl.cxx @@ -19,6 +19,8 @@ #include +#include + #include #include #include @@ -91,6 +93,10 @@ struct UndoTableCpyTable_Entry // Was the last paragraph of the new and the first paragraph of the old content joined? bool bJoin; // For redlining only + // Was this box copied as a change? A box that holds a formula is not, see lcl_CpyBox(), + // and then undo and redo have to take the same route the copy took. + bool bTrackChanges; + explicit UndoTableCpyTable_Entry( const SwTableBox& rBox ); void dumpAsXml(xmlTextWriterPtr pWriter) const; @@ -2360,7 +2366,7 @@ void SwUndoTableNumFormat::SetBox( const SwTableBox& rBox ) UndoTableCpyTable_Entry::UndoTableCpyTable_Entry( const SwTableBox& rBox ) : nBoxIdx( rBox.GetSttIdx() ), nOffset( 0 ), - bJoin( false ) + bJoin( false ), bTrackChanges( true ) { } @@ -2397,6 +2403,11 @@ void UndoTableCpyTable_Entry::dumpAsXml(xmlTextWriterPtr pWriter) const BAD_CAST(OString::boolean(bJoin).getStr())); (void)xmlTextWriterEndElement(pWriter); + (void)xmlTextWriterStartElement(pWriter, BAD_CAST("bTrackChanges")); + (void)xmlTextWriterWriteAttribute(pWriter, BAD_CAST("value"), + BAD_CAST(OString::boolean(bTrackChanges).getStr())); + (void)xmlTextWriterEndElement(pWriter); + (void)xmlTextWriterEndElement(pWriter); } @@ -2411,6 +2422,39 @@ SwUndoTableCpyTable::~SwUndoTableCpyTable() m_pInsRowUndo.reset(); } +namespace +{ +// Move cursors off the temporary node at the start of the box, onto the content that stays. +// +// UndoImpl() and RedoImpl() park cursors on that node while they hand the content of the box to +// an undo object, the same way AddBoxBefore() does, and the node is deleted right after, so the +// cursors have to come off it first. This is what AddBoxAfter() does at the end of a copy. +void lcl_MoveCursorsOffTempNode(const SwTableBox & rBox) +{ + SwPaM aPam(SwNodeIndex(*rBox.GetSttNd(), 1)); + if (!aPam.GetPoint()->GetNode().IsTextNode()) + return; + aPam.SetMark(); + aPam.Move(fnMoveForward, GoInContent); + ::PaMCorrAbs(aPam, *aPam.GetPoint()); +} + +// Put the cursor at the start of the content of the box that starts at nBoxStart. +void lcl_SetCursorToBoxContent(::sw::UndoRedoContext & rContext, + const SwTableNode *const pTableNd, SwNodeOffset const nBoxStart) +{ + if (!pTableNd) + return; + const SwTableBox *const pBox = pTableNd->GetTable().GetTableBox(nBoxStart); + if (!pBox || !pBox->GetSttNd()) + return; + SwPaM & rPaM(rContext.GetCursorSupplier().CreateNewShellCursor()); + rPaM.DeleteMark(); + rPaM.GetPoint()->Assign(*pBox->GetSttNd()); + rPaM.Move(fnMoveForward, GoInContent); +} +} + void SwUndoTableCpyTable::UndoImpl(::sw::UndoRedoContext & rContext) { SwDoc & rDoc = rContext.GetDoc(); @@ -2436,14 +2480,22 @@ void SwUndoTableCpyTable::UndoImpl(::sw::UndoRedoContext & rContext) SwTableBox& rBox = *pBox; SwNodeIndex aInsIdx( *rBox.GetSttNd(), 1 ); - rDoc.GetNodes().MakeTextNode( aInsIdx.GetNode(), rDoc.GetDfltTextFormatColl() ); + SwTextNode *const pNewNode( + rDoc.GetNodes().MakeTextNode( aInsIdx.GetNode(), rDoc.GetDfltTextFormatColl() )); // b62341295: Redline for copying tables const SwNode *pEndNode = rBox.GetSttNd()->EndOfSectionNode(); SwPaM aPam( aInsIdx.GetNode(), *pEndNode ); std::unique_ptr pUndo; - if( IDocumentRedlineAccess::IsRedlineOn( GetRedlineFlags() ) ) + { // move cursors to the new node which precedes aPam, the way AddBoxBefore() does: + // the content below is about to be handed to an undo object, and a cursor left in it + // still points there once that object lets the nodes go. + SwPosition const pos(*pNewNode, 0); + ::PaMCorrAbs(aPam, pos); + } + + if( IDocumentRedlineAccess::IsRedlineOn( GetRedlineFlags() ) && pEntry->bTrackChanges ) { bool bDeleteCompleteParagraph = false; bool bShiftPam = false; @@ -2465,8 +2517,14 @@ void SwUndoTableCpyTable::UndoImpl(::sw::UndoRedoContext & rContext) SwTextNode *pText = aTmpIdx.GetNode().GetTextNode(); if( pText ) { + // The content start was recorded when the copy was done. The text of a + // box can be rebuilt after that, by the number format of the box + // regenerating it from the value, and that leaves the recorded offset + // describing text that is no longer there. Keep it inside the text, the + // same way the redline table clamps its own stale positions. aPam.GetPoint()->Assign(*pText, - pUndoRedlineDelete->ContentStart() ); + std::min(pUndoRedlineDelete->ContentStart(), + pText->GetText().getLength()) ); } else aPam.GetPoint()->Assign( aTmpIdx ); @@ -2510,6 +2568,7 @@ void SwUndoTableCpyTable::UndoImpl(::sw::UndoRedoContext & rContext) pEntry->pUndo = std::move(pUndo); aInsIdx = rBox.GetSttIdx() + 1; + lcl_MoveCursorsOffTempNode( rBox ); rDoc.GetNodes().Delete( aInsIdx ); SfxItemSet aTmpSet(SfxItemSet::makeFixedSfxItemSetUndoImpl(rContext); } + + if (!m_vArr.empty()) + lcl_SetCursorToBoxContent(rContext, pTableNd, m_vArr[0]->nBoxIdx + m_vArr[0]->nOffset); + DEBUG_REDLINE( rDoc ) } @@ -2570,15 +2633,24 @@ void SwUndoTableCpyTable::RedoImpl(::sw::UndoRedoContext & rContext) SwNodeIndex aInsIdx( *rBox.GetSttNd(), 1 ); // b62341295: Redline for copying tables - Start. - rDoc.GetNodes().MakeTextNode( aInsIdx.GetNode(), rDoc.GetDfltTextFormatColl() ); + SwTextNode *const pNewNode( + rDoc.GetNodes().MakeTextNode( aInsIdx.GetNode(), rDoc.GetDfltTextFormatColl() )); SwPaM aPam( aInsIdx.GetNode(), *rBox.GetSttNd()->EndOfSectionNode()); - std::unique_ptr pUndo(IDocumentRedlineAccess::IsRedlineOn(GetRedlineFlags()) + const bool bRedlineBox = IDocumentRedlineAccess::IsRedlineOn(GetRedlineFlags()) + && pEntry->bTrackChanges; + if( !bRedlineBox ) + { // as in UndoImpl() above: take cursors out of the content that the undo object below + // is about to take over + SwPosition const pos(*pNewNode, 0); + ::PaMCorrAbs(aPam, pos); + } + std::unique_ptr pUndo(bRedlineBox ? nullptr : std::make_unique(aPam, SwDeleteFlags::Default, true)); if( pEntry->pUndo ) { pEntry->pUndo->UndoImpl(rContext); - if( IDocumentRedlineAccess::IsRedlineOn( GetRedlineFlags() ) ) + if( bRedlineBox ) { // PrepareRedline has to be called with the beginning of the old content // When new and old content has been joined, the rIter.pAktPam has been set @@ -2603,6 +2675,7 @@ void SwUndoTableCpyTable::RedoImpl(::sw::UndoRedoContext & rContext) // b62341295: Redline for copying tables - End. aInsIdx = rBox.GetSttIdx() + 1; + lcl_MoveCursorsOffTempNode( rBox ); rDoc.GetNodes().Delete( aInsIdx ); SfxItemSet aTmpSet(SfxItemSet::makeFixedSfxItemSetnOffset = rBox.GetSttIdx() - pEntry->nBoxIdx; } + + if (!m_vArr.empty()) + lcl_SetCursorToBoxContent(rContext, pTableNd, m_vArr[0]->nBoxIdx + m_vArr[0]->nOffset); + DEBUG_REDLINE( rDoc ) } -void SwUndoTableCpyTable::AddBoxBefore( const SwTableBox& rBox, bool bDelContent ) +void SwUndoTableCpyTable::AddBoxBefore( const SwTableBox& rBox, bool bDelContent, + bool bTrackChanges ) { if (!m_vArr.empty() && !bDelContent) return; UndoTableCpyTable_Entry* pEntry = new UndoTableCpyTable_Entry( rBox ); + pEntry->bTrackChanges = bTrackChanges; m_vArr.push_back(std::unique_ptr(pEntry)); SwDoc& rDoc = rBox.GetFrameFormat()->GetDoc(); @@ -2649,7 +2728,7 @@ void SwUndoTableCpyTable::AddBoxBefore( const SwTableBox& rBox, bool bDelContent SwTextNode *const pNewNode(rDoc.GetNodes().MakeTextNode(aInsIdx.GetNode(), rDoc.GetDfltTextFormatColl())); SwPaM aPam( aInsIdx.GetNode(), *rBox.GetSttNd()->EndOfSectionNode() ); - if( !rDoc.getIDocumentRedlineAccess().IsRedlineOn() ) + if( !bTrackChanges || !rDoc.getIDocumentRedlineAccess().IsRedlineOn() ) { { // move cursors to new node which precedes aPam SwPosition const pos(*pNewNode, 0); @@ -2670,7 +2749,8 @@ void SwUndoTableCpyTable::AddBoxBefore( const SwTableBox& rBox, bool bDelContent DEBUG_REDLINE( rDoc ) } -void SwUndoTableCpyTable::AddBoxAfter( const SwTableBox& rBox, const SwNodeIndex& rIdx, bool bDelContent ) +void SwUndoTableCpyTable::AddBoxAfter( const SwTableBox& rBox, const SwNodeIndex& rIdx, + bool bDelContent, bool bTrackChanges ) { UndoTableCpyTable_Entry *const pEntry = m_vArr.back().get(); @@ -2687,7 +2767,7 @@ void SwUndoTableCpyTable::AddBoxAfter( const SwTableBox& rBox, const SwNodeIndex pam.Move(fnMoveForward, GoInContent); ::PaMCorrAbs(pam, *pam.GetPoint()); } - if( rDoc.getIDocumentRedlineAccess().IsRedlineOn() ) + if( bTrackChanges && rDoc.getIDocumentRedlineAccess().IsRedlineOn() ) { SwPosition aTmpPos( rIdx ); pEntry->pUndo = PrepareRedline( rDoc, rBox, aTmpPos, pEntry->bJoin, false );