bug 58618: XWPFParagraph insertNewRun and removeRun work incorrectly for

runs after hyperlink/field runs

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1722503 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 50 insertions, 8 deletions

diff --git a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java
index 4f29318865..a81ab3b1e8 100644
--- a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java
+++ b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFParagraph.java
@@ -1332,13 +1332,26 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
     /**
      * insert a new Run in RunArray
      *
-     * @param pos
-     * @return the inserted run
+     * @param pos The position at which the new run should be added.
+     * 
+     * @return the inserted run or null if the given pos is out of bounds.
      */
     public XWPFRun insertNewRun(int pos) {
-        if (pos >= 0 && pos <= paragraph.sizeOfRArray()) {
-            CTR ctRun = paragraph.insertNewR(pos);
-            XWPFRun newRun = new XWPFRun(ctRun, (IRunBody)this);
+        if (pos >= 0 && pos <= runs.size()) {
+            // calculate the correct pos as our run/irun list contains
+            // hyperlinks
+            // and fields so it is different to the paragraph R array.
+            int rPos = 0;
+            for (int i = 0; i < pos; i++) {
+                XWPFRun currRun = runs.get(i);
+                if (!(currRun instanceof XWPFHyperlinkRun
+                        || currRun instanceof XWPFFieldRun)) {
+                    rPos++;
+                }
+            }
+
+            CTR ctRun = paragraph.insertNewR(rPos);
+            XWPFRun newRun = new XWPFRun(ctRun, (IRunBody) this);
 
             // To update the iruns, find where we're going
             // in the normal runs, and go in there
@@ -1357,6 +1370,7 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
 
             return newRun;
         }
+
         return null;
     }
     // TODO Add methods to allow adding a HyperlinkRun or a FieldRun
@@ -1376,6 +1390,7 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
         int beginRunPos = 0, candCharPos = 0;
         boolean newList = false;
         
+        @SuppressWarnings("deprecation")
         CTR[] rArray = paragraph.getRArray();
         for (int runPos = startRun; runPos < rArray.length; runPos++) {
             int beginTextPos = 0, beginCharPos = 0, textPos = 0, charPos = 0;
@@ -1444,8 +1459,10 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
         int textEnd = segment.getEndText();
         int charEnd = segment.getEndChar();
         StringBuilder out = new StringBuilder();
+        @SuppressWarnings("deprecation")
         CTR[] rArray = paragraph.getRArray();
         for (int i = runBegin; i <= runEnd; i++) {
+            @SuppressWarnings("deprecation")
             CTText[] tArray = rArray[i].getTArray();
             int startText = 0, endText = tArray.length - 1;
             if (i == runBegin)
@@ -1473,7 +1490,7 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
      * @return true if the run was removed
      */
     public boolean removeRun(int pos) {
-        if (pos >= 0 && pos < paragraph.sizeOfRArray()) {
+        if (pos >= 0 && pos < runs.size()) {
             // Remove the run from our high level lists
             XWPFRun run = runs.get(pos);
             if (run instanceof XWPFHyperlinkRun ||
@@ -1485,7 +1502,14 @@ public class XWPFParagraph implements IBodyElement, IRunBody, ISDTContents, Para
             runs.remove(pos);
             iruns.remove(run);
             // Remove the run from the low-level XML
-            getCTP().removeR(pos);
+            //calculate the correct pos as our run/irun list contains hyperlinks and fields so is different to the paragraph R array.
+            int rPos = 0;
+            for(int i=0;i<pos;i++) {
+              XWPFRun currRun = runs.get(i);
+              if(!(currRun instanceof XWPFHyperlinkRun || currRun instanceof XWPFFieldRun))
+                rPos++;
+            }
+            getCTP().removeR(rPos);
             return true;
         }
         return false;
diff --git a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFBugs.java b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFBugs.java
index cc7b7c0650..6d780e89ff 100644
--- a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFBugs.java
@@ -51,6 +51,8 @@ public class TestXWPFBugs {
         assertEquals(run.getFontFamily(FontCharRange.hAnsi), "Times New Roman");
         run.setFontFamily("Arial", FontCharRange.hAnsi);
         assertEquals(run.getFontFamily(FontCharRange.hAnsi), "Arial");
+        
+        doc.close();
     }
 
 
@@ -101,4 +103,20 @@ public class TestXWPFBugs {
             assertNotNull(paragraph.getText());
         }
     }
+    
+  /**
+   * Removing a run needs to take into account position of run if paragraph contains hyperlink runs
+   */
+  @Test
+  public void test58618() throws Exception {
+      XWPFDocument doc = XWPFTestDataSamples.openSampleDocument("58618.docx");
+      XWPFParagraph para = (XWPFParagraph)doc.getBodyElements().get(0);
+      assertNotNull(para);
+      assertEquals("Some text  some hyper links link link and some text.....", para.getText());
+      XWPFRun run = para.insertNewRun(para.getRuns().size());
+      run.setText("New Text");
+      assertEquals("Some text  some hyper links link link and some text.....New Text", para.getText());
+      para.removeRun(para.getRuns().size() -2);
+      assertEquals("Some text  some hyper links link linkNew Text", para.getText());
+  }
 }
diff --git a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFParagraph.java b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFParagraph.java
index 915f0716b5..d9fede9b2c 100644
--- a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFParagraph.java
+++ b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFParagraph.java
@@ -280,7 +280,7 @@ public final class TestXWPFParagraph {
         assertEquals(0, paragraph.getCTP().sizeOfBookmarkEndArray());
         CTBookmark ctBookmark = paragraph.getCTP().getBookmarkStartArray(0);
         assertEquals("poi", ctBookmark.getName());
-        for (CTBookmark bookmark : paragraph.getCTP().getBookmarkStartArray()) {
+        for (CTBookmark bookmark : paragraph.getCTP().getBookmarkStartList()) {
             assertEquals("poi", bookmark.getName());
         }
         doc.close();
diff --git a/test-data/document/58618.docx b/test-data/document/58618.docx
new file mode 100644
index 0000000000..5a779391e1
--- /dev/null
+++ b/test-data/document/58618.docx