Merge pull request #184 from ruby-docx/fix-111-bookmark-outside-paragraph

Insert bookmark text into a paragraph for block-level bookmarks (#111)

Author: satoryu

lib/docx/elements/bookmark.rb

@@ -51,28 +51,92 @@ 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)
-        else
+        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
+          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)
-        else
+        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
+          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. 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
+        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
+
+      # 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 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
+
+        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 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
+
+        Containers::TextRun.create_within(Containers::Paragraph.new(paragraph_node))
+      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

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 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', 'bookmark_1'])
+    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')