[Bug-1630262], [Bug-1615425]: segfault when deleting lines or tagging outside of the -startline/-endline range with peer text widgets. [Bug-3487407]: Weird text indices.

8 files changed, 224 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index 5ef99bf..5360007 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-02-28  Francois Vogel  <fvogelnew1@free.fr>
+
+	* generic/tkText.c:      [Bug-1630262], [Bug-1615425]: segfault
+	* generic/tkTextBTree.c  when deleting lines or tagging outside of
+	* generic/tkTextDisp.c   the -startline/-endline range with peer
+	* generic/tkTextMark.c   text widgets.
+	* tests/text.test        [Bug-3487407]: Weird text indices.
+	* tests/textMark.test
+
 2012-02-28  Donal K. Fellows  <dkf@users.sf.net>
 
 	* doc/canvas.n: [Bug 3495198]: Corrected types of bitmap options.
diff --git a/generic/tkText.c b/generic/tkText.c
index d050170..6652f3d 100644
--- a/generic/tkText.c
+++ b/generic/tkText.c
@@ -2107,6 +2107,10 @@ ConfigureText(
 	 * Also, clamp the insert and current (unshared) marks to the new
 	 * -startline/-endline range limits of the widget. All other (shared)
 	 * marks are unchanged.
+         * The return value of TkTextMarkNameToIndex does not need to be
+         * checked: "insert" and "current" marks always exist, and the
+         * purpose of the code below precisely is to move them inside the
+         * -startline/-endline range.
 	 */
 
 	textPtr->sharedTextPtr->stateEpoch++;
@@ -3097,6 +3101,11 @@ DeleteIndexRange(
 		resetView = 1;
 		line = line1;
 		byteIndex = tPtr->topIndex.byteIndex;
+            } else {
+                /*
+                 * Deletion range starts after the top line. This peers's view
+                 * will not need to be reset. Nothing to do.
+                 */
 	    }
 	} else if (index2.linePtr == tPtr->topIndex.linePtr) {
 	    /*
@@ -3113,6 +3122,11 @@ DeleteIndexRange(
 	    } else {
 		byteIndex -= (index2.byteIndex - index1.byteIndex);
 	    }
+        } else {
+            /*
+             * Deletion range ends before the top line. This peers's view
+             * will not need to be reset. Nothing to do.
+             */
 	}
 	if (resetView) {
 	    lineAndByteIndex[resetViewCount] = line;
@@ -3157,14 +3171,43 @@ DeleteIndexRange(
 	    TkTextIndex indexTmp;
 
 	    if (tPtr == textPtr) {
-		if (viewUpdate) {
+                if (viewUpdate) {
+                    /*
+                     * line cannot be before -startline of textPtr because
+                     * this line corresponds to an index which is necessarily
+                     * between "1.0" and "end" relative to textPtr.
+                     * Therefore no need to clamp line to the -start/-end
+                     * range.
+                     */
+
 		    TkTextMakeByteIndex(sharedTextPtr->tree, textPtr, line,
 			    byteIndex, &indexTmp);
 		    TkTextSetYView(tPtr, &indexTmp, 0);
 		}
 	    } else {
-		TkTextMakeByteIndex(sharedTextPtr->tree, NULL, line,
+                TkTextMakeByteIndex(sharedTextPtr->tree, tPtr, line,
 			byteIndex, &indexTmp);
+                /*
+                 * line may be before -startline of tPtr and must be
+                 * clamped to -startline before providing it to
+                 * TkTextSetYView otherwise lines before -startline
+                 * would be displayed.
+                 * There is no need to worry about -endline however,
+                 * because the view will only be reset if the deletion
+                 * involves the TOP line of the screen
+                 */
+
+                if (tPtr->start != NULL) {
+                    int start;
+                    TkTextIndex indexStart;
+
+                    start = TkBTreeLinesTo(NULL, tPtr->start);
+                    TkTextMakeByteIndex(sharedTextPtr->tree, NULL, start,
+			    0, &indexStart);
+                    if (TkTextIndexCmp(&indexTmp, &indexStart) < 0) {
+                        indexTmp = indexStart;
+                    }
+                }
 		TkTextSetYView(tPtr, &indexTmp, 0);
 	    }
 	}
diff --git a/generic/tkTextBTree.c b/generic/tkTextBTree.c
index 0038e64..67ff79d 100644
--- a/generic/tkTextBTree.c
+++ b/generic/tkTextBTree.c
@@ -662,12 +662,12 @@ AdjustStartEndRefs(
 	if (textPtr->start != NULL) {
 	    count--;
 	    treePtr->startEnd[count] = textPtr->start;
-	    treePtr->startEndRef[count] = treePtr->sharedTextPtr->peers;
+	    treePtr->startEndRef[count] = textPtr;
 	}
 	if (textPtr->end != NULL) {
 	    count--;
 	    treePtr->startEnd[count] = textPtr->end;
-	    treePtr->startEndRef[count] = treePtr->sharedTextPtr->peers;
+	    treePtr->startEndRef[count] = textPtr;
 	}
     }
 }
@@ -1609,7 +1609,7 @@ TkBTreeFindLine(
     }
 
     /*
-     * Check for the any start/end offset for this text widget.
+     * Check for any start/end offset for this text widget.
      */
 
     if (textPtr != NULL) {
@@ -1991,12 +1991,37 @@ TkBTreeLinesTo(
 	    index += nodePtr2->numLines;
 	}
     }
-    if (textPtr != NULL && textPtr->start != NULL) {
-	index -= TkBTreeLinesTo(NULL, textPtr->start);
+    if (textPtr != NULL) {
+        /* 
+         * The index to return must be relative to textPtr, not to the entire
+         * tree. Take care to never return a negative index when linePtr
+         * denotes a line before -startline, or an index larger than the
+         * number of lines in textPtr when linePtr is a line past -endline.
+         */
+
+        int indexStart, indexEnd;
+
+        if (textPtr->start != NULL) {
+            indexStart = TkBTreeLinesTo(NULL, textPtr->start);
+        } else {
+            indexStart = 0;
+        }
+        if (textPtr->end != NULL) {
+            indexEnd = TkBTreeLinesTo(NULL, textPtr->end);
+        } else {
+            indexEnd = TkBTreeNumLines(textPtr->sharedTextPtr->tree, NULL);
+        }
+        if (index < indexStart) {
+            index = 0;
+        } else if (index > indexEnd) {
+            index = TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr);
+        } else {
+            index -= indexStart;
+        }
     }
     return index;
 }
-
+
 /*
  *----------------------------------------------------------------------
  *
diff --git a/generic/tkTextDisp.c b/generic/tkTextDisp.c
index 5f30b11..d0cd4d2 100644
--- a/generic/tkTextDisp.c
+++ b/generic/tkTextDisp.c
@@ -1971,7 +1971,7 @@ UpdateDisplayInfo(
 
 	if (spaceLeft <= dInfoPtr->newTopPixelOffset) {
 	    /*
-	     * We can full up all the needed space just by showing more of the
+	     * We can fill up all the needed space just by showing more of the
 	     * current top line.
 	     */
 
@@ -3229,7 +3229,7 @@ TextInvalidateLineMetrics(
 	 */
 
 	TkBTreeLinePixelEpoch(textPtr, linePtr) = 0;
-	while (counter > 0 && linePtr != 0) {
+	while (counter > 0 && linePtr != NULL) {
 	    linePtr = TkBTreeNextLine(textPtr, linePtr);
 	    if (linePtr != NULL) {
 		TkBTreeLinePixelEpoch(textPtr, linePtr) = 0;
@@ -3244,7 +3244,7 @@ TextInvalidateLineMetrics(
 	 * more lines than is strictly necessary (but the examination of the
 	 * extra lines should be quick, since their pixelCalculationEpoch will
 	 * be up to date). However, to keep track of that would require more
-	 * complex record-keeping that what we have.
+	 * complex record-keeping than what we have.
 	 */
 
 	if (dInfoPtr->lineUpdateTimer == NULL) {
diff --git a/generic/tkTextMark.c b/generic/tkTextMark.c
index a9d709c..71a7949 100644
--- a/generic/tkTextMark.c
+++ b/generic/tkTextMark.c
@@ -285,6 +285,7 @@ TkTextSetMark(
 
 	if (markPtr == textPtr->insertMarkPtr) {
 	    TkTextIndex index, index2;
+            int nblines;
 
 	    TkTextMarkSegToIndex(textPtr, textPtr->insertMarkPtr, &index);
 	    TkTextIndexForwChars(NULL,&index, 1, &index2, COUNT_INDICES);
@@ -295,8 +296,17 @@ TkTextSetMark(
 	     */
 
 	    TkTextChanged(NULL, textPtr, &index, &index2);
-	    if (TkBTreeLinesTo(textPtr, indexPtr->linePtr) ==
-		    TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr))  {
+
+            /*
+             * The number of lines in the widget is zero if and only if it is
+             * a partial peer with -startline == -endline, i.e. an empty
+             * peer. In this case the mark shall be set exactly at the given
+             * index, and not one character backwards (bug 3487407).
+             */
+
+	    nblines = TkBTreeNumLines(textPtr->sharedTextPtr->tree, textPtr);
+	    if ((TkBTreeLinesTo(textPtr, indexPtr->linePtr) == nblines)
+		    && (nblines > 0))  {
 		TkTextIndexBackChars(NULL,indexPtr, 1, &insertIndex,
 			COUNT_INDICES);
 		indexPtr = &insertIndex;
@@ -385,9 +395,15 @@ TkTextMarkSegToIndex(
  *
  * Results:
  *	The return value is TCL_OK if "name" exists as a mark in the text
- *	widget. In this case *indexPtr is filled in with the next segment
- *	whose after the mark whose size is non-zero. TCL_ERROR is returned if
- *	the mark doesn't exist in the text widget.
+ *	widget and is located within its -starline/-endline range. In this
+ *	case *indexPtr is filled in with the next segment who is after the
+ *	mark whose size is non-zero. TCL_ERROR is returned if the mark
+ *	doesn't exist in the text widget, or if it is out of its -starline/
+ *	-endline range. In this latter case *indexPtr still contains valid
+ *	information, in particular TkTextMarkNameToIndex called with the
+ *	"insert" or "current" mark name may return TCL_ERROR, but *indexPtr
+ *	contains the correct index of this mark before -startline or after
+ *	-endline.
  *
  * Side effects:
  *	None.
diff --git a/tests/text.test b/tests/text.test
index 44a3065..52689ba 100644
--- a/tests/text.test
+++ b/tests/text.test
@@ -11,7 +11,7 @@ eval tcltest::configure $argv
 tcltest::loadTestedCommands
 namespace import -force tcltest::test
 
-# Create entries in the option database to be sure that geometry options
+# Create entries in the odeption database to be sure that geometry options
 # like border width have predictable values.
 
 option add *Text.borderWidth 2
@@ -3622,6 +3622,100 @@ test text-32.1 {peer widget -start, -end and selection} {
     set res
 } {{10.0 20.0} {6.0 16.0} {6.0 11.0} {1.0 6.0} {1.0 2.0} {} {10.0 20.0}}
 
+test text-32.2 {peer widget -start, -end and deletion (bug 1630262)} -setup {
+    destroy .t .pt
+    set res {}
+} -body {
+    text .t
+    .t peer create .pt
+    for {set i 1} {$i < 100} {incr i} {
+      .t insert end "Line $i\n"
+    }
+    .t configure -startline 5
+    # none of the following delete shall crash
+    # (all did before fixing bug 1630262)
+    # 1. delete on the same line: line1 == line2 in DeleteIndexRange,
+    #    and resetView is true neither for .t not for .pt
+    .pt delete 2.0 2.2
+    # 2. delete just one line: line1 < line2 in DeleteIndexRange,
+    #    and resetView is true only for .t, not for .pt
+    .pt delete 2.0 3.0
+    # 3. delete several lines: line1 < line2 in DeleteIndexRange,
+    #    and resetView is true only for .t, not for .pt
+    .pt delete 2.0 5.0
+    # 4. delete to the end line: line1 < line2 in DeleteIndexRange,
+    #    and resetView is true only for .t, not for .pt
+    .pt delete 2.0 end
+    # this test succeeds provided there is no crash
+    set res 1
+} -cleanup {
+    destroy .pt
+} -result {1}
+
+test text-32.3 {peer widget -start, -end and deletion (bug 1630262)} -setup {
+    destroy .t .pt
+    set res {}
+} -body {
+    text .t
+    .t peer create .pt
+    for {set i 1} {$i < 100} {incr i} {
+      .t insert end "Line $i\n"
+    }
+    .t configure -startline 5
+    .pt configure -startline 3
+    # the following delete shall not crash
+    # (it did before fixing bug 1630262)
+    .pt delete 2.0 3.0
+    # moreover -startline shall be correct
+    # (was wrong before fixing bug 1630262)
+    lappend res [.t cget -start] [.pt cget -start]
+} -cleanup {
+    destroy .pt
+} -result {4 3}
+
+test text-32.4 {peer widget -start, -end and deletion (bug 1630262)} -setup {
+    destroy .t .pt
+    set res {}
+} -body {
+    text .t
+    .t peer create .pt
+    for {set i 1} {$i < 100} {incr i} {
+      .t insert end "Line $i\n"
+    }
+    .t configure -startline 5 -endline 15
+    .pt configure -startline 8 -endline 12
+    # .pt now shows a range entirely inside the range of .pt
+    # from .t, delete lines located after [.pt cget -end]
+    .t delete 9.0 10.0
+    # from .t, delete lines straddling [.pt cget -end]
+    .t delete 6.0 9.0
+    lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end]
+    .t configure -startline 5 -endline 12
+    .pt configure -startline 8 -endline 12
+    # .pt now shows again a range entirely inside the range of .pt
+    # from .t, delete lines located before [.pt cget -start]
+    .t delete 2.0 3.0
+    # from .t, delete lines straddling [.pt cget -start]
+    .t delete 2.0 5.0
+    lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end]
+    .t configure -startline 22 -endline 31
+    .pt configure -startline 42 -endline 51
+    # .t now shows a range entirely before the range of .pt
+    # from .t, delete some lines, then do it from .pt
+    .t delete 2.0 3.0
+    .t delete 2.0 5.0
+    .pt delete 2.0 5.0
+    lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end]
+    .t configure -startline 55 -endline 75
+    .pt configure -startline 60 -endline 70
+    # .pt now shows a range entirely inside the range of .t
+    # from .t, delete a range straddling the entire range of .pt
+    .t delete 3.0 18.0
+    lappend res [.t cget -start] [.t cget -end] [.pt cget -start] [.pt cget -end]
+} -cleanup {
+    destroy .pt
+} -result {5 11 8 10 5 8 6 8 22 27 38 44 55 60 57 57}
+
 test text-33.1 {widget dump -command alters tags} {
     .t delete 1.0 end
     .t insert end "abc\n" a "---" {} "def" b "   \n" {} "ghi\n" c
diff --git a/tests/textMark.test b/tests/textMark.test
index c1138ac..67b9ae5 100644
--- a/tests/textMark.test
+++ b/tests/textMark.test
@@ -170,6 +170,13 @@ test textMark-6.4 {TkTextMarkNameToIndex, with mark outside -startline/-endline
   .pt configure -startline {} -endline {}
   .t mark unset mymark
 } -result {1 {bad text index "mymark"} 1.0 1.0 1 {bad text index "mymark"} L 1 {bad text index "mymark"}}
+test textMark-6.5 {insert and current marks in an empty peer - bug 3487407} -body {
+  .t mark set insert 1.0
+  .t configure -start 5 -end 5
+  set res [.t index insert]
+} -cleanup {
+  .t configure -startline {} -endline {}
+} -result {1.0}
 
 catch {eval {.t mark unset} [.t mark names]}
 test textMark-7.1 {MarkFindNext - invalid mark name} haveCourier12 {
diff --git a/tests/textTag.test b/tests/textTag.test
index b112fc2..dcec25d 100644
--- a/tests/textTag.test
+++ b/tests/textTag.test
@@ -147,6 +147,19 @@ test textTag-2.13 {TkTextTagCmd - "add" option} haveCourier12 {
     .t tag add sel 1.1 1.5 2.4
     .t tag ranges sel
 } {1.1 1.5 2.4 2.5}
+test textTag-2.14 {tag add before -startline - Bug 1615425} haveCourier12 {
+    text .tt
+    for {set i 1} {$i <10} {incr i} {
+        .tt insert end "Line $i\n"
+    }
+    .tt tag configure mytag -font {Courier 12 bold}
+    .tt peer create .ptt
+    .ptt configure -startline 3 -endline 7
+    # the test succeeds if next line does not crash
+    .tt tag add mytag 1.0 1.end
+    destroy .ptt .tt
+    set res 1
+} {1}
 
 catch {.t tag delete x}
 test textTag-3.1 {TkTextTagCmd - "bind" option} haveCourier12 {