From 27efd7f536a22f89769e0c794d6c9e8ad4fa9416 Mon Sep 17 00:00:00 2001 From: Tatsuya Sato Date: Mon, 1 Jun 2026 23:52:27 +0900 Subject: [PATCH 1/2] Insert bookmark text into a paragraph for block-level bookmarks (#111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some generators (notably Google Docs) place w:bookmarkStart/w:bookmarkEnd directly under w:body, outside any w:p. get_run_before/after only looked at sibling runs and, finding none, created a run as a sibling of the bookmark — i.e. outside any paragraph. Runs outside a w:p are not rendered by Word and are invisible to #paragraphs, so insert_text_before/after silently lost the text. Make run placement paragraph-aware: for a block-level bookmark, insert_text_after prepends to the following paragraph and insert_text_before appends to the preceding one (creating a new paragraph when there is no adjacent one), so the run always lives inside a w:p. parent_paragraph now falls back to the adjacent paragraph too, so insert_multiple_lines works as well. Bookmarks inside a paragraph are unchanged. Adds a fixture reproducing the Google Docs layout (from #111) and specs for insert_text_after/before, insert_multiple_lines, and a save round-trip. Co-Authored-By: Claude Opus 4.8 --- lib/docx/elements/bookmark.rb | 72 +++++++++++++++++- spec/docx/document_spec.rb | 42 ++++++++++ spec/fixtures/bookmark_outside_paragraph.docx | Bin 0 -> 13482 bytes 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/bookmark_outside_paragraph.docx diff --git a/lib/docx/elements/bookmark.rb b/lib/docx/elements/bookmark.rb index b524d17..1f02ccc 100644 --- a/lib/docx/elements/bookmark.rb +++ b/lib/docx/elements/bookmark.rb @@ -57,10 +57,16 @@ def get_run_before if not (r_nodes = @node.xpath("./preceding-sibling::w:r")).empty? r_node = r_nodes.last Containers::TextRun.new(r_node) - else + elsif enclosing_paragraph new_r = Containers::TextRun.create_with(self) new_r.insert_before(self) new_r + else + # Block-level bookmark (e.g. Google Docs places bookmarkStart/End + # directly under w:body). Append to the preceding paragraph, or start a + # new paragraph before the bookmark, so the run lives inside a w:p. + run_at_paragraph_end(@node.xpath("./preceding-sibling::w:p").last) || + run_in_new_paragraph { |paragraph| @node.add_previous_sibling(paragraph) } end end @@ -68,12 +74,74 @@ def get_run_before def get_run_after if (r_node = @node.at_xpath("./following-sibling::w:r")) Containers::TextRun.new(r_node) - else + elsif enclosing_paragraph new_r = Containers::TextRun.create_with(self) new_r.insert_after(self) new_r + else + # Block-level bookmark: prepend to the following paragraph, or start a + # new paragraph after the bookmark. + run_at_paragraph_start(@node.at_xpath("./following-sibling::w:p")) || + run_in_new_paragraph { |paragraph| @node.add_next_sibling(paragraph) } end end + + # Override Element#parent_paragraph so insert_multiple_lines also works for + # block-level bookmarks: fall back to the adjacent paragraph. + def parent_paragraph + node = enclosing_paragraph || + @node.at_xpath("./following-sibling::w:p") || + @node.xpath("./preceding-sibling::w:p").last + Containers::Paragraph.new(node) + end + + private + + # The w:p the bookmark sits inside, or nil when it is block-level. + def enclosing_paragraph + @node.at_xpath("./parent::w:p") + end + + # A run at the start of paragraph_node (reusing its first run to keep + # formatting, or creating one). nil when paragraph_node is nil. + def run_at_paragraph_start(paragraph_node) + return nil unless paragraph_node + + if (existing = paragraph_node.at_xpath("w:r")) + return Containers::TextRun.new(existing) + end + + new_r = Containers::TextRun.create_with(self) + if (props = paragraph_node.at_xpath("w:pPr")) + props.add_next_sibling(new_r.node) + else + paragraph_node.prepend_child(new_r.node) + end + new_r + end + + # A run at the end of paragraph_node. nil when paragraph_node is nil. + def run_at_paragraph_end(paragraph_node) + return nil unless paragraph_node + + if (existing = paragraph_node.xpath("w:r").last) + return Containers::TextRun.new(existing) + end + + new_r = Containers::TextRun.create_with(self) + paragraph_node.add_child(new_r.node) + new_r + end + + # Create a run wrapped in a fresh w:p; the block positions that paragraph + # relative to the bookmark. + def run_in_new_paragraph + new_r = Containers::TextRun.create_with(self) + paragraph = Nokogiri::XML::Node.new("w:p", @node.document) + paragraph.add_child(new_r.node) + yield paragraph + new_r + end end end end \ No newline at end of file diff --git a/spec/docx/document_spec.rb b/spec/docx/document_spec.rb index 949f3ce..9ac3d70 100755 --- a/spec/docx/document_spec.rb +++ b/spec/docx/document_spec.rb @@ -153,6 +153,48 @@ end end + # Regression test for #111: some generators (e.g. Google Docs) place + # w:bookmarkStart/End directly under w:body, outside any w:p. Inserting text at + # such a bookmark must land inside a paragraph (otherwise the run is orphaned + # and silently lost). + describe 'bookmarks placed outside a paragraph (e.g. Google Docs)' do + before do + @doc = Docx::Document.open(@fixtures_path + '/bookmark_outside_paragraph.docx') + @new_path = @fixtures_path + '/bookmark_outside_paragraph_saved.docx' + end + + after do + File.delete(@new_path) if File.exist?(@new_path) + end + + it 'finds the block-level bookmark' do + expect(@doc.bookmarks['bookmark_1']).to_not be_nil + expect(@doc.paragraphs.map(&:text)).to eq(['bookmark_1']) + end + + it 'inserts text after the bookmark into the following paragraph' do + @doc.bookmarks['bookmark_1'].insert_text_after('INSERTED ') + expect(@doc.paragraphs.map(&:text)).to eq(['INSERTED bookmark_1']) + end + + it 'inserts text before the bookmark as a new leading paragraph' do + @doc.bookmarks['bookmark_1'].insert_text_before('BEFORE') + expect(@doc.paragraphs.map(&:text)).to eq(['BEFORE', 'bookmark_1']) + end + + it 'inserts multiple lines at the bookmark' do + @doc.bookmarks['bookmark_1'].insert_multiple_lines(['line1', 'line2', 'line3']) + expect(@doc.paragraphs.map(&:text)).to eq(['line1', 'line2', 'line3']) + end + + it 'persists inserted text after save' do + @doc.bookmarks['bookmark_1'].insert_text_after('INSERTED ') + @doc.save(@new_path) + reopened = Docx::Document.open(@new_path) + expect(reopened.paragraphs.map(&:text)).to eq(['INSERTED bookmark_1']) + end + end + describe 'read tables' do before do @doc = Docx::Document.open(@fixtures_path + '/tables.docx') diff --git a/spec/fixtures/bookmark_outside_paragraph.docx b/spec/fixtures/bookmark_outside_paragraph.docx new file mode 100644 index 0000000000000000000000000000000000000000..b6c75e2e2459c93cec92a9db310738ff4fb4ab6d GIT binary patch literal 13482 zcmeHuWmKHomTln{+}+*Xf=h7MAi-UOJ0VDLcemi~?!h&<1_>545S*ebx%b?Y@cQ=a zKd;}P9)nS}>l923C+fFxJ|03GyHN6g;N z+0@SYjhcspsgoX)yRFSY602e#3rdh11yF3KL1xue7?$%GU$w07DTP6G? zko>U%C4mZ@EJU^h zj0)1)rOQi`S}m^+a3!{5+M)Z*Jkhm*GW}y7)oQf5#gH?$T5p+m>a@U#kweUN`N9oY z2bSNjm1&h9l4>~!Exd9RMvNeq zE2iIov2kg-v zufe~ozs_j^0x7t-2FXe;n0SaSql1YfI&=~m_zK?%K+XlDHV|D3?^)y!1aYU6RJsOXN-yNU9Z*aS38VvW{b3< z{lS1|!@^9N5o=Tqk9XESedw9rRWuM9!~mBI7s`C^ZKjL%8n_p>iz~!+N~&G|X44)& zLr>%a?9Vqsj;8J}4;t6~&;S4d=#9A9zcpe0$7?aMH+HcF4e_62``0UKNE}xH4e-EI zsdlj)e#h52P_y;e!iSh)JZ%$vp;GO!9S@hstc>!X>$M10x1+w^N*WKQj&&Z=-2sec zXKC^K;lhPw$7C{#j!e(p9s9^+Ma}a>5ovYanbsZLF73e4iP*(6j(n1wkXT8;6dxzt z)4))EU4w^uB|K76cmLumM^+s<*2K zBli){HG^9dpc(1HDCfIwh>x1z2vpEjS5aaj%e*T%!M#jwH-+p#wtg!ViYx>l3X^GB zVCP@4WlLfK1<%RYUwfz$XqfAP)Xx}$2ed(T0FKEZF_xtUacZVN3 zNt%k7^L&VI*1OE_5dRDV;rvZeDkuzsPyhhxUt#z?0EG$T3g1BFa_ti~bU!~lhbgM8 zisis;gjgQ?tvPaT(KvD~uv1|BAwalzDsj+tf_sp?W`VovqXJ%#{(DBPMj7qA@<5dp zpPsC2Aaj-W*8A$h;-1og%mj1p^nuoK*_c=PHc}r9eAfB2tLY$?ZIS$)Z;TuZ(&tD6c6lzYy-_9CcHnsQ%&M>^OuB7+seX!7m}}owicSx z)H=T@espQoFWeJlaRFA7@j^m+9Um`P`Q6^mF{PUrRBHr*x;#<$olM&;PFLZy8K-F4 zDbjRGnmpqWP=EeCKL#Tr`x(K#DKneP>4TS1Qr2|rhZKxGt(z1lXQRMIr>!@MyFQNB z$#YjkoBL*Ioe@_C$msOzqr}z0L>)qDU1MX$lmvPt6dK;#Ze9vN0h(fXl8HkZ2Fe< zXC4(Mtk|uvya+ypxg>nCMqL}yh!ep#FrO2WZfMa4qh!j9MHdQAxiY#EEl5erhoS9c zyj*Z|j%|5xco)qnUB+VJ05zIKE1M9nWYt8wc)GVTz?~FGqf7gON4>fN{>ani>TBs} z*#|h&wiq{ktn#Soy$l#_divfOt5B4jFj=@HF?GulcNnejRhCIe_CzA3D8;jp4i&IA zy-U{|XCunh;PqrP)OO97H_JCr(;Xk~B=8BF`W2YXUtpArBrARxP*BQ_iw>Y-H~+%w zGMKCO3G zOQS5S%>#gTs*nvqjyr3?2fnOJKMI{`fz5cPqWeN`2CXz;b4zpk(BjeSG8qmkBJtpe zH#{jt&75ILjlCF0`k^YDjKXR|?!<$RxVO45>g6CyN*X4YOJg3?^uEJtG?j_swa1L*mr4T`w;52pt7z%V|-Z6c-Qni#ulcACZ=!M{>Nhc4Gn$7 zmGrcoL5nbdk{7vFLnE+o4vkb8GZ~E`q;y8*9I1Ne&1VL}^_Kx^q)H|Do3Bw91M5yL zza-`xKv_r>D6TrtpEaB#qiP&VH{Kx{dhYx;GcEBVlbVtNmr#49u8my*7`RNCxxPbn#d zujC3|9kE5lo;B{4_ro7w1qsci_3hdzd^;u--M;4V!8`8hIMI4ceL$Y)$nUEN`rQ9= zuJ27JXAiJ~^#QYlnX&p(i6+zSQSzY`u68V2F$p4$j|GyIXe~zsGH#D)h8S8vqI^Ozjv|a#Z1b8v>svnTerD0b7qUU3Vmo^>w!>q-@lric@G{nKYDV1|* zy$Qd5=~W-?5YYSz?i9)<(40TxR!*$UKLd)79Gr124S#dpil4L4ndurF!x{D*%} z#2nHLp%ELrmDr5_lpP+~Dy_$T4CTl&4XmEziJAl+ig(GeFaAbhY#jkfx^enqN?0VL zROQ6Co|(|#j{;{Hw;seMrTVvyMrhenu~@8<8DyKr6C$Cgcvqoa>S=vjm6K)1`F+bL zmUl0g(c2@-ugriD)(arYzP;#wncO=A#m46I8iKE;R0>u7rIjLu4naGXa(r+FWtFA| z59$;v+rdlY_=5ZCnYgU{2e(gK?jBAq+_H-XuS-Q|)o+e&=h9A2%opm2ww4XYy0M+s zmn`bnc#3aTQpI8M4o{ebo+{XYlVn|Tu5xERz05$spQV@#(cioj6x`O|r_H8zCcnk@ zuld%|KjvE>7bAF}mJP&C!(6-o>Pg1AShP}a+uMB(oD^Ya%((etb_d@S6#keKQkC%f z+AcYyRU4sQAZ%1=m|WB4=gZVtHv|LLQjDVZ32CIRY4ey^#x)@?<&{SUOquQ| zs270sBri`4-Y`UFe1%;i=I>uuv#}C+>!^?1sKCK7-mSW`=Mz)n`E@W<+FSJ1>wXN! zu)t*ASCo<#Dd>Ks9FyT~{8f!z1oN&R4=ya4&Uk8;!*9Xnr}Fn+UF?OnFb5SPn3#CB zK{#*al1ZqY7eA6OnfWs0Q6a*}DpZv_kmVSg%y@jllD^I?WE7CRJ&hPRzZx9)L1&n6 zSq87HxmmGS$*H$d&ZH^O-_V!v&VIhVZrY;F5#=I_DHMfcc1@zwuYwggiMAo{Dt^*_ zl}`OC@vy? zm}`Y8uXvpFJR%|$3czQ@h|TxPGkoO)ih z`;hFoX4mxD#q5C%aP_i!R=aoJOTI|pYstIsqo(VbVLBzr=Xg)cXukPtc5V2S-nLOy z%$w9&4c7zioM}?ex0v4{YJ6V z>->zKUH|ZA!|(2>=}i|0L?V(m@W-}LmY^?ie9sSfHU-$ga?k0z=O9B{BN^QHe`Sy& z0>J%Hr+g_U=Mr|z6T|a~HWF-2-4M>lH~^RH%W}a{&kFc!e>is+@1}w?Lz+)vc>ZmM zYTzf16U(|dw#)LS2l_aJ@3fn7zBNNcM9bCkBb*;cp%0l_87;v{OON|NlMaG~hKe2t z2$J;+yN1lfy}2%?Zr%|KI#HVaG`k-sNJzsz&50+XOGO1mx=y)qv?gggA$R09q>Kxv z{w_6?(vXr{f5141JpU7q5#M({ja=#BzR)unq-1z9LX=O+3)CuU7ItCM(rbDzI)-qr z1h_&+q;mI%0^6f_7h?InXW<+Za9ln*78?u2kxMB@T(z*#^^9W1Rgxm{=;NNOBox=p zT`}SX?m|kRMo3II$rAG9om(EKDDMyMtwrU62|is-KNX|e_puMF!G#~YVcOLkxHg|1 ztZX>Bo$;(w$LyZ!oZKhfGc~TN|7f?%&%D;qAr52rogRVnkiz9xLzk=^EvJ`NT0ZSD zYaIGu2H}C$Yf4%fhyXcJ8T;iBy|lLvg_`8kFa>0-0ijfa1Kc3Mn)39BLAvM0W-^lq z02Vn%bbWE0=F5^%GpzA$7HUof`Wt+io+rDZ0+Oz3+ICuDOOZ?+r@bgANe0ntO`+ET@VSG;?r-&{qJFAp)qi!}=YMl1O}q1j&j7mT<_p+y=;W61*of*Rrk!9Z5L z>KdNz95;bUrfeB^GqoZFFwED?XjQtm5||8~*|en)brk&)idI%NlbcIFLT38zAAC4HHjQzW5#;218g5Y_bb14e+6}!L041;%|BuJE^86B+b zs%gIEIMv~eZXbrq<;wv>!{Ez=ha$Z%t-in<%P0kUXR^`Vjmhby?sP%C71t7~SeLZw z3hK4wrX`5o#CmU*UTTeUxxO`A)M@mc*XLylLTR=sR*ALz8!rr-_EdOt?^%s;EX}nB z;s!VZL8UvW$aMJV@x2v6A^-jSCGk)aRA!Fw@9DNk!h0XhbVm+12=jMIEh;};dmv?} z)mhVL__Q`JxHVdzHF&j_9e+h@Xm~Hz`1)=k6!|3Jh>(XSGBxftyuiz0_DJrD86bF- z;rxyGPtxZbc8wtck$&5Y-!{RVOr4!A?aY78)G{@-omRL|d{5<`-Um-6x`?}~$R|WZ z(YP895`9* zi57i;!oyq3b%T=7_CrpfELgk=GT-gZ=Jm$9{UBb12-lhjrf77TcU;1!*=>ErJxVq4 z>|s2}0QwR-qcc^^j<8}ERW~Q?x&G=1N3*LA!!EyANY1x$95NB_!|Vp$Qa0$~(z7L% zVUChMh{OHBYtuI&z4!=jm@$wA9wjK1E35IXxZ zxnzbVr({W_?rGZ-W*~7c$Tdqyp{MdH-#KZ1=Y^juka)p5m2}8Y%`s zwJ(IJ7Eo34qqV|SMW+P)EgZkMUz!lzBW4rgqjDdjG8DQe?cBTW3L~ip4-=v%7lWeP zRj8ztmmIGdKafUcBttTT+q0V(MU2cWbGL z*VmNl(SD__$b-sTkb;U-*U>{dX9B#;R90=w)rVN>eqpWcls8&Jc#34hqru}O0nK<_ zq!%em!nicD`o1UjD~afnWqpeoIMX}K!WJ~-#{3Q<#ElxLnuCz2lr({m`{0mH+JsdF zd~(e(@vAD=w+@3LV0!GfQ8K48N*VB~j}1MRIV{~%PRiaR`*3aY2+EB z72yZ2iE$%dPWzS2bst@!w&h#CP6hPnFmTwih=CW!Bi#wykR;1l)|v3FLI}1tCBW4iV13bp3LdlU3vLF$1#~?`}dab)Ip* zAL(JBe7ENtIvxvto6U8CeC`sq68MGpE3FhA-X7mvr(yHXF;82@k3dd;33`?e9xi0= zhsXkh5Uc#fZwm@&ULGv4j8hr{otni_Azn7sMY+?&?SzQCsvFW+K8WA#fV*o(zVNBz zlgtw^MKJ@}`AZ&`3^%Hg$MoHe#X?J_b0zKl8EUCU1y05rZ~~*TB|lya>UFbskHdvX zEt{YoxeDaeh}@vTyn(#Sva_}Tqj%f&l@!`mqAqFf0D8`tCNmXJHq%;1+hLaY(2FPa zSYS}3c6U~h>2Yi06+)ieLBJwBx#sI;EnnIULfaGD_*P1{jR-pvt7ac%&~s-vZqGI_ zmzU$nq1o+?bSCUPRCXo@9u+Hrb zxI`cs$0Uu2@bt20IUx&WoxZgWw-0GUOE|TM+IQSS!u4vHv%R_zf_s8;DTQ1=TzmIU z3xY{KVCy6+I#AE=<;8$t`dhP*$Ig-ELZ99IT^?N3V#j&h60)mT`}H?f+kMaWDf4VT z$DV@b&e@;^($9m;Uq?G`rbep2F>2&gk4!%c3iOw@FZ>CsO_mbkGAmHAP?Luc1qbzr zW^9I3)_U3NN}3|=M~q@-f@VP1r+1z1Fj%j0Rb8}8zyk`WgIZLpm_yb&=qOTI@D#!L zT)>s@fp?{P>6pRhQnYR|v56#z%L1_cE() z0N+n(Q{~bo-lkkg;wKJ%v+~N-gj(LUMCvy?3XUwJQ%^JZe~yK z2d`%--UXVZxzb#|Pi>UNPwCLGp`166)t9E4deapr6zA{g9iiYQ!EoO`Swqnkm%80_ zN<7owR;0q(vj_Y#UURBU6!zi#$7)x{t+Vs4mkc^lthfW;HF3%JjjsFqs;2bRnP@om zEYvSnjl;by#MYVP0m4D7@}ZvbwMy`m8h7$EGs5K>3MrfqRubqqDk_*B^|9JD+jfodj0o%4dogauY$~8DEHt&9u>iG`=!g%Ai;}`B?jz;w(s0}SXJxHsT}}&jKKI&TFIP%fxf|$x zo91exO_3|CBChP^TPW*3NShJmaAfFJw8WUCrbNg^d$~&9okpmOleHJn$5Fpt67ygz z>pLvIxsZ0KXdjO#j;X;7}p|1Fwcm6=%FKe(`uLhu3I4(cO1&=^`II zBQ^$;pmt{VCI**|K8=OHyKlHz@FsNyNZEpgNl`{}Un`RTIZ^=EOFQAC?ZnKSChGdM zMecZL%bRMs?c=!y6Hz?s%=^!lbIm!l_q}-^mO1k}V+I-Wo<1dP(BT~~`+3u4jmfZL z@N##BY)iAEqG9@Y!&;hU&SZS;j~aHvk6A36BPxQOTw+SE-NbFMc&L&@Dn0iMMN>7> z59S z2#c{;P$OX<)bIy2{M{ruJ~grDv1Y)Ki>eTC8!7ZSeF;7Ybj6$Mo<5{oxvmVQu$Hb} zth2U$|5n&AmYI`b?ww940%On`v8zF_828v9M|h}FDgMsl4{q1%ZJ#8DL-EYUa`xk` z1vofMhPV$hr@?~zo#z`tO*C$}YuktOh;Nqo=ChV?BvBTd3qQAAcCi(<4d~bv$`Wt{ zZzp*yF##hb85F-&f*nbIB7fCijqmtfCXf&?#F){#5AmJ2OJyU9mEtKhT#)369s5za z$=AN9h2lHXmAkyBP#ZQG>f1zW1Fw`z-+(6AMaE0t4^4L@@jriNFBZ8G1@Dd{vMCaJ z*TW3-D0wMmWH!KzE*W`MueLYB6gM3J3$Eqn?pO5t%C4@`GD9O@djeXUZekJEI#>z$ zItSi3%D3bZMR2`c0N->X#~Flv@iw<2v@y9-$c0{sDT-^O*{g7tHb2w z?=Ual4MZ?pgH%G9mj^)Eu0d8$&5v&Yqh=s>u4-o>0*%)JC{Z3ySJx>-ByJZc!Cz9j z{e%AT5JH$|7ty^%g2f&H)zc1!!EpgyJvD~k-OFkZfE{cHBO$ne_IDV=2lTR@Zrp9q z@IX$sgE9Q}FkZ-L9RMHT0YPK2i)fJh%ramzg&Xj*|ClxpH?gxn4GTV!?;_@qq;UHO zn5_e_(mWuvK+gON0&@A6cYv03H}2=%o)d!{{J(i@4;Wf&ZzFXFIrf(yO7`~EE{$+M zy1$k{6qG`{3zS>?*n7MeItY^)5fi2a&D%Qrh{Ok%w~$3wJ*s<8lt&@K@)y|*^JwZk zY%CO!5-FdYHhpqe-nN-zo+5fzdwg$+No4H2;=n!}9NRa&pA%Hpw$sN%#KzW_Sef*+ zsh0086U-@13)iK$l4bx;FJ|E?S5ox=sAIOso7VJKk>bl zTFi~b-j==xc}G_*jQEJWs=0u%=Kv!qci22NAv-0NSo0+tc)Hb0VyYx^e-(3kj`a7) z1kLdB$CMy4)B| zAsdl5G#j>8s<9$G%O(1$1tWphlrNL2P+!KqG4X(yD^NwDP{}BFBV|GjRh#y6`$pX> zOEhR&ffije(Krty=tIUCHB(j%164 zsr#H~s5U@wd`+>DmDq6AYD+aHw^6Puu}i1pjdckVZoaIW65LVE(B9G>rz=&b@^RBd z1#?1cgxQ^-&S-ibvko(Q$&Ws1jRs37!H>VGPLEclW@7BzfTK}0w)o~Vp{YlW>)2dG z0rX>oMGQv69C?&&=PvSe0+g4U&W&H>ktX>)hUtKmV>*-%wM75~-pi5|X`_8+N6_C6 zGwb(coO@Y>#?*(!rd5{MT{5ty^s)VN&Jx=OcHo41c>Tc!m5;T;Lx}GyL?0%~!!s|vKw%8BF zX+{lU7RNvb9Q4LU3z7h*5q^a82KNhHO~ds29pwu-e{*w9i{3p(b@htm-)J)j5q^zA z_Z7>DH;g@MHIs8^=ug7OaqE2VcErw8INRPKGG7uB?V96`V+9k|ROf2op<@G+l)c~z zaN$}zJxVIO!)^L}S^8=+eWE*;<{jTP#xaV4U{IgCXS_Y(yI>gg z$kmBN(nVMeF=S7F1n|7gf(^N`k0R9aZ8L$1*>GI~3#_vT&+0B<>9RZ$Kf0fxE}g3yseu-G7VlZ_}Ib1I+*M>3Uy%3}0B}XPFuY6`-GG z>K|3z3(J zV)Ofc(-DtE4jX#?Pb-REVV@*WM1HiWs@oV>6kj#p;mAIg(<32`${g;cE;D*v;wBVw zZs*gZlg!BpqQ6O`?UL86_4s~2-B|E>rJG2yrc!Twtq+gp+wqv&rS5906tN$bS-L*( zs2jRbM#n!MjzJWC@+B>GKY^}U4eFvAeM1CCC%K{Cc?9J!w#qz~hu2&nqq9fD2EXst zovsDr%+=M5Abumfcw3M7ib8#~Tcg6BoQ9jkH@-&gg7HnJI$42NtpbgiUJ;dvxOiUt zCxe=+3vyWq$_&?!G^61?Zz#r}kV^TYK3rI>59_LTxghcbq4=>!W{IObe^jruPB+?) zh9qJ$w3w!;s#vJKkv~bm+>jN-ShQesqzuwpG=6O}>cS#$4^s!RujGqz?qdvVn`=C# z-_*zKGJGf1asO;V#F&Ix?gCmZ9)jeNKUHWT0h-cVdj}_GWBa$JKWmRqoU&{m3r5fe z)RWk-SF*DUUEyeVs3H<1b(lSCi_Z2k1%+nYqt|Ds|Tp=O)Li&OqXJ0BHqVYu0q^U4wJ=b##5pi`yMf5U&Le8=e`YS)jqpHd^c41lLpnnFrxeJ6z^MF~@If98U$6eo&^ zi1UI2-}I6{{7d@Tp77~|6c?Tw5-@^joU{Yc6SmQo-|9jT?|O27bPHJXG^dtcNe>s8 zEQ3{C1FbKjw-Sfc>eM^9!Y{S+cuum_bV)Mh3G`$==*T}gT<@-I&=2Bq$up7@iP7he zNNzUIBItK!1ZUc+sN^V4C(PE=hpDdw-{|vQS_=$OToZuLx(*dd3JJb0dnaSWKF~Rs ze~~e1pX?SGt<9ydB#~A_nU29rkK=XR$e;QI%@qO#q~0Zr)ngpgX1$eo34R5Y*{J}0 z79;D?4$WRrj4J;;&$e?`Gc>aK6{L>%wW!Y^so-g>d&toB3XdgC+w{k>LZu!=Yzq&- zf=e1IF!ZIJ*hW@pJamCdjYL@_rlq%;w~Zg(hR_2CA1_Vx7n^}vT=US{8QY!{x~oS^ zAE!~Y7rk~tt{Un`#W4a58=WnuBg;$va1~8jc-l2ofsJyi36X5z=9TJ6aSazBE#RG= zRp3*Ny?p_D1#C4K;%3Tj2J@oDcJ}txAzs1V{?0lL{?^?ry?oS!5d>q_YU^IJ%Y?no zX@O_~Kfb^XE|_cWa2Pu|#0U2FSL~ zz>966;{*+B&QWe;pmJf=_wuRX&JhGw;!IjH++=iZLp}0JK*`>VF2P3A$pv}GSQ`ox zE0#ugvQCS}j7tl_B0u&7WAsL$d99v>`D|tEMStVOPbfE{S6Sb{S}NXo)IV>-PrNNS z(~q5|R;cTFC7E3kd?g2iW-cyB@1q1((A>QRQA0x(Z>ZY`&M)kZi*>PJP=kiYMZ^@N zHE8$YMKQJhd&tQr3gwuy=Cp(jidw(;=C-szQj}!RJU{Hddo5l0t*7-tUO4@SAV@zj zG-LnKH-BzZq@ppRTZ1A#0?Nvtn!hiwog za96XEp|nftkp8JW*AsurlK&hG;Pzfl{Sjq0AG|U-Z$gZd=jxiING*|-o2X!^Gx^=c z!_PI-oE5Gt&=Jf07RlImR8i3YH)Ak{<5L8MO~zMw8DMY?4Ze*;hSG{k(c4_iP=n&s zGCD@b&7DSx^hFP4UDgLV3(Qh>A=urHt?^fPRmE>>FRO0vQ8uoB2z+Qs=Rp6e zf?#8i>nxq3(lmsS2Lq5*Hr`P+@Bzceo+Y}58pB5ol&cpKq;!Scobbvol&aSP7MMCr ztJ8WdwT-tr8f?=G!)eeoi4^IX8@mw$>AndF7>hKf~B6Pw>$#!zQ)b6pS)m43WoC)KmXeV?;G|91SMH8a1Mxne=7hK9RMg&fP!~FTmQ)& zf#(LE3(|kW{$bz~=<7Fm`g7oO-S*$WMbI$$uTrwxsX=R)Uyl5ye|?UAuBH4NjR3l6@eBQ%vhq3l-}ik6_ebBqBzw=%|Gw{8(A|LF z`u-(0c#eLqEBf2>7R2BB{+ANvIsUnH zxryh;ZGW3E!TzU--w)rO!=G<2{|&DLT_pJB!L!Zg=N6vtzy57umhdkJ{sk drr)~%C)F#-LV=R%r;rT|;19}943?k2{sY>y8omGk literal 0 HcmV?d00001 From 43e2b7bff65da43c17fbea0eeb377e550992e77e Mon Sep 17 00:00:00 2001 From: Tatsuya Sato Date: Tue, 2 Jun 2026 00:02:00 +0900 Subject: [PATCH 2/2] Address review on #111 block-level bookmark fix - Always create a new run for block-level insertion instead of reusing the adjacent paragraph's existing run, which silently dropped the text when that run had multiple w:t nodes (TextRun#text= is a no-op for those). - Check enclosing_paragraph first in get_run_before/after so a stray following/preceding w:r outside a paragraph is no longer picked up (the inserted text would have gone into an orphan, invisible run). - parent_paragraph now fills a fresh paragraph inserted at the bookmark for block-level bookmarks, instead of an adjacent existing one. This avoids both a crash (no adjacent paragraph) and overwriting unrelated content in insert_multiple_lines. - Reuse TextRun.create_within for run_at_paragraph_end. Co-Authored-By: Claude Opus 4.8 --- lib/docx/elements/bookmark.rb | 80 +++++++++++++++++------------------ spec/docx/document_spec.rb | 4 +- 2 files changed, 40 insertions(+), 44 deletions(-) diff --git a/lib/docx/elements/bookmark.rb b/lib/docx/elements/bookmark.rb index 1f02ccc..01f9171 100644 --- a/lib/docx/elements/bookmark.rb +++ b/lib/docx/elements/bookmark.rb @@ -51,48 +51,54 @@ def insert_multiple_lines(text_array) # Get text run immediately prior to bookmark node def get_run_before - # at_xpath returns the first match found and preceding-sibling returns siblings in the - # order they appear in the document not the order as they appear when moving out from - # the starting node - if not (r_nodes = @node.xpath("./preceding-sibling::w:r")).empty? - r_node = r_nodes.last - Containers::TextRun.new(r_node) - elsif enclosing_paragraph + if enclosing_paragraph + # at_xpath returns the first match found and preceding-sibling returns siblings in the + # order they appear in the document not the order as they appear when moving out from + # the starting node + unless (r_nodes = @node.xpath("./preceding-sibling::w:r")).empty? + return Containers::TextRun.new(r_nodes.last) + end + new_r = Containers::TextRun.create_with(self) new_r.insert_before(self) - new_r - else - # Block-level bookmark (e.g. Google Docs places bookmarkStart/End - # directly under w:body). Append to the preceding paragraph, or start a - # new paragraph before the bookmark, so the run lives inside a w:p. - run_at_paragraph_end(@node.xpath("./preceding-sibling::w:p").last) || - run_in_new_paragraph { |paragraph| @node.add_previous_sibling(paragraph) } + return new_r end + + # Block-level bookmark (e.g. Google Docs places bookmarkStart/End directly + # under w:body). Add a run to the preceding paragraph, or to a new + # paragraph before the bookmark, so the run lives inside a w:p. + run_at_paragraph_end(@node.xpath("./preceding-sibling::w:p").last) || + run_in_new_paragraph { |paragraph| @node.add_previous_sibling(paragraph) } end # Get text run immediately after bookmark node def get_run_after - if (r_node = @node.at_xpath("./following-sibling::w:r")) - Containers::TextRun.new(r_node) - elsif enclosing_paragraph + if enclosing_paragraph + if (r_node = @node.at_xpath("./following-sibling::w:r")) + return Containers::TextRun.new(r_node) + end + new_r = Containers::TextRun.create_with(self) new_r.insert_after(self) - new_r - else - # Block-level bookmark: prepend to the following paragraph, or start a - # new paragraph after the bookmark. - run_at_paragraph_start(@node.at_xpath("./following-sibling::w:p")) || - run_in_new_paragraph { |paragraph| @node.add_next_sibling(paragraph) } + return new_r end + + # Block-level bookmark: add a run to the following paragraph, or to a new + # paragraph after the bookmark. + run_at_paragraph_start(@node.at_xpath("./following-sibling::w:p")) || + run_in_new_paragraph { |paragraph| @node.add_next_sibling(paragraph) } end # Override Element#parent_paragraph so insert_multiple_lines also works for - # block-level bookmarks: fall back to the adjacent paragraph. + # block-level bookmarks. For those we fill a fresh paragraph inserted at the + # bookmark, rather than an adjacent existing paragraph, so we neither crash + # (no enclosing paragraph) nor overwrite unrelated content. def parent_paragraph - node = enclosing_paragraph || - @node.at_xpath("./following-sibling::w:p") || - @node.xpath("./preceding-sibling::w:p").last - Containers::Paragraph.new(node) + return Containers::Paragraph.new(enclosing_paragraph) if enclosing_paragraph + + paragraph = Nokogiri::XML::Node.new("w:p", @node.document) + @node.add_next_sibling(paragraph) + Containers::Paragraph.new(paragraph) end private @@ -102,15 +108,11 @@ def enclosing_paragraph @node.at_xpath("./parent::w:p") end - # A run at the start of paragraph_node (reusing its first run to keep - # formatting, or creating one). nil when paragraph_node is nil. + # A new run inserted at the start of paragraph_node (after w:pPr if present). + # Returns nil when paragraph_node is nil. def run_at_paragraph_start(paragraph_node) return nil unless paragraph_node - if (existing = paragraph_node.at_xpath("w:r")) - return Containers::TextRun.new(existing) - end - new_r = Containers::TextRun.create_with(self) if (props = paragraph_node.at_xpath("w:pPr")) props.add_next_sibling(new_r.node) @@ -120,17 +122,11 @@ def run_at_paragraph_start(paragraph_node) new_r end - # A run at the end of paragraph_node. nil when paragraph_node is nil. + # A new run appended to the end of paragraph_node. nil when it is nil. def run_at_paragraph_end(paragraph_node) return nil unless paragraph_node - if (existing = paragraph_node.xpath("w:r").last) - return Containers::TextRun.new(existing) - end - - new_r = Containers::TextRun.create_with(self) - paragraph_node.add_child(new_r.node) - new_r + Containers::TextRun.create_within(Containers::Paragraph.new(paragraph_node)) end # Create a run wrapped in a fresh w:p; the block positions that paragraph diff --git a/spec/docx/document_spec.rb b/spec/docx/document_spec.rb index 9ac3d70..7e204c6 100755 --- a/spec/docx/document_spec.rb +++ b/spec/docx/document_spec.rb @@ -182,9 +182,9 @@ expect(@doc.paragraphs.map(&:text)).to eq(['BEFORE', 'bookmark_1']) end - it 'inserts multiple lines at the bookmark' do + it 'inserts multiple lines at the bookmark without destroying the following paragraph' do @doc.bookmarks['bookmark_1'].insert_multiple_lines(['line1', 'line2', 'line3']) - expect(@doc.paragraphs.map(&:text)).to eq(['line1', 'line2', 'line3']) + expect(@doc.paragraphs.map(&:text)).to eq(['line1', 'line2', 'line3', 'bookmark_1']) end it 'persists inserted text after save' do