Fixed bug [1700065] - error in trace proc on textvariable doesn't trigger bgerror

3 files changed, 108 insertions, 37 deletions

diff --git a/generic/tkEntry.c b/generic/tkEntry.c
index 036d1e2..e1189b6 100644
--- a/generic/tkEntry.c
+++ b/generic/tkEntry.c
@@ -391,7 +391,7 @@ static const char *const selElementNames[] = {
 
 static int		ConfigureEntry(Tcl_Interp *interp, Entry *entryPtr,
 			    int objc, Tcl_Obj *const objv[], int flags);
-static void		DeleteChars(Entry *entryPtr, int index, int count);
+static int		DeleteChars(Entry *entryPtr, int index, int count);
 static void		DestroyEntry(void *memPtr);
 static void		DisplayEntry(ClientData clientData);
 static void		EntryBlinkProc(ClientData clientData);
@@ -417,7 +417,7 @@ static int		EntryValidateChange(Entry *entryPtr, const char *change,
 static void		ExpandPercents(Entry *entryPtr, const char *before,
 			    const char *change, const char *newStr, int index,
 			    int type, Tcl_DString *dsPtr);
-static void		EntryValueChanged(Entry *entryPtr,
+static int		EntryValueChanged(Entry *entryPtr,
 			    const char *newValue);
 static void		EntryVisibleRange(Entry *entryPtr,
 			    double *firstPtr, double *lastPtr);
@@ -427,7 +427,7 @@ static int		EntryWidgetObjCmd(ClientData clientData,
 static void		EntryWorldChanged(ClientData instanceData);
 static int		GetEntryIndex(Tcl_Interp *interp, Entry *entryPtr,
 			    const char *string, int *indexPtr);
-static void		InsertChars(Entry *entryPtr, int index, const char *string);
+static int		InsertChars(Entry *entryPtr, int index, const char *string);
 
 /*
  * These forward declarations are the spinbox specific ones:
@@ -664,7 +664,7 @@ EntryWidgetObjCmd(
 	break;
 
     case COMMAND_DELETE: {
-	int first, last;
+	int first, last, code;
 
 	if ((objc < 3) || (objc > 4)) {
 	    Tcl_WrongNumArgs(interp, 2, objv, "firstIndex ?lastIndex?");
@@ -681,7 +681,10 @@ EntryWidgetObjCmd(
 	    goto error;
 	}
 	if ((last >= first) && (entryPtr->state == STATE_NORMAL)) {
-	    DeleteChars(entryPtr, first, last - first);
+	    code = DeleteChars(entryPtr, first, last - first);
+            if (code != TCL_OK) {
+                goto error;
+            }
 	}
 	break;
     }
@@ -722,7 +725,7 @@ EntryWidgetObjCmd(
     }
 
     case COMMAND_INSERT: {
-	int index;
+	int index, code;
 
 	if (objc != 4) {
 	    Tcl_WrongNumArgs(interp, 2, objv, "index text");
@@ -733,7 +736,10 @@ EntryWidgetObjCmd(
 	    goto error;
 	}
 	if (entryPtr->state == STATE_NORMAL) {
-	    InsertChars(entryPtr, index, Tcl_GetString(objv[3]));
+	    code = InsertChars(entryPtr, index, Tcl_GetString(objv[3]));
+            if (code != TCL_OK) {
+                goto error;
+            }
 	}
 	break;
     }
@@ -1298,6 +1304,9 @@ ConfigureEntry(
     /*
      * If the entry is tied to the value of a variable, create the variable if
      * it doesn't exist, and set the entry's value from the variable's value.
+     * No checking of the return value of EntryValueChanged is needed since it
+     * can only return TCL_ERROR if there is a trace on the textvariable and
+     * since any existing trace on it was eliminated above.
      */
 
     if (entryPtr->textVarName != NULL) {
@@ -1993,7 +2002,8 @@ EntryComputeGeometry(
  *	Add new characters to an entry widget.
  *
  * Results:
- *	None.
+ *	A standard Tcl result. If an error occurred then an error message is
+ *	left in the interp's result.
  *
  * Side effects:
  *	New information gets added to entryPtr; it will be redisplayed soon,
@@ -2002,12 +2012,12 @@ EntryComputeGeometry(
  *----------------------------------------------------------------------
  */
 
-static void
+static int
 InsertChars(
     Entry *entryPtr,		/* Entry that is to get the new elements. */
     int index,			/* Add the new elements before this character
 				 * index. */
-    const char *value)	/* New characters to add (NULL-terminated
+    const char *value)		/* New characters to add (NULL-terminated
 				 * string). */
 {
     ptrdiff_t byteIndex;
@@ -2020,7 +2030,7 @@ InsertChars(
     byteIndex = Tcl_UtfAtIndex(string, index) - string;
     byteCount = strlen(value);
     if (byteCount == 0) {
-	return;
+	return TCL_OK;
     }
 
     newByteCount = entryPtr->numBytes + byteCount + 1;
@@ -2034,7 +2044,7 @@ InsertChars(
 	    EntryValidateChange(entryPtr, value, newStr, index,
 		    VALIDATE_INSERT) != TCL_OK) {
 	ckfree(newStr);
-	return;
+	return TCL_OK;
     }
 
     ckfree((char *)string);
@@ -2082,7 +2092,7 @@ InsertChars(
     if (entryPtr->insertPos >= index) {
 	entryPtr->insertPos += charsAdded;
     }
-    EntryValueChanged(entryPtr, NULL);
+    return EntryValueChanged(entryPtr, NULL);
 }
 
 /*
@@ -2093,7 +2103,8 @@ InsertChars(
  *	Remove one or more characters from an entry widget.
  *
  * Results:
- *	None.
+ *	A standard Tcl result. If an error occurred then an error message is
+ *	left in the interp's result.
  *
  * Side effects:
  *	Memory gets freed, the entry gets modified and (eventually)
@@ -2102,7 +2113,7 @@ InsertChars(
  *----------------------------------------------------------------------
  */
 
-static void
+static int
 DeleteChars(
     Entry *entryPtr,		/* Entry widget to modify. */
     int index,			/* Index of first character to delete. */
@@ -2116,7 +2127,7 @@ DeleteChars(
 	count = entryPtr->numChars - index;
     }
     if (count <= 0) {
-	return;
+	return TCL_OK;
     }
 
     string = entryPtr->string;
@@ -2138,7 +2149,7 @@ DeleteChars(
 		    VALIDATE_DELETE) != TCL_OK) {
 	ckfree(newStr);
 	ckfree(toDelete);
-	return;
+	return TCL_OK;
     }
 
     ckfree(toDelete);
@@ -2197,7 +2208,7 @@ DeleteChars(
 	    entryPtr->insertPos = index;
 	}
     }
-    EntryValueChanged(entryPtr, NULL);
+    return EntryValueChanged(entryPtr, NULL);
 }
 
 /*
@@ -2210,7 +2221,8 @@ DeleteChars(
  *	is one, and does other bookkeeping such as arranging for redisplay.
  *
  * Results:
- *	None.
+ *	A standard Tcl result. If an error occurred then an error message is
+ *	left in the interp's result.
  *
  * Side effects:
  *	None.
@@ -2218,7 +2230,7 @@ DeleteChars(
  *----------------------------------------------------------------------
  */
 
-static void
+static int
 EntryValueChanged(
     Entry *entryPtr,		/* Entry whose value just changed. */
     const char *newValue)	/* If this value is not NULL, we first force
@@ -2232,7 +2244,7 @@ EntryValueChanged(
 	newValue = NULL;
     } else {
 	newValue = Tcl_SetVar2(entryPtr->interp, entryPtr->textVarName,
-		NULL, entryPtr->string, TCL_GLOBAL_ONLY);
+		NULL, entryPtr->string, TCL_GLOBAL_ONLY|TCL_LEAVE_ERR_MSG);
     }
 
     if ((newValue != NULL) && (strcmp(newValue, entryPtr->string) != 0)) {
@@ -2254,6 +2266,17 @@ EntryValueChanged(
 	EntryComputeGeometry(entryPtr);
 	EventuallyRedraw(entryPtr);
     }
+
+    /*
+     * An error may have happened when setting the textvariable in case there
+     * is a trace on that variable and the trace proc triggered an error.
+     * Signal this error.
+     */
+
+    if ((entryPtr->textVarName != NULL) && (newValue == NULL)) {
+        return TCL_ERROR;
+    }
+    return TCL_OK;
 }
 
 /*
@@ -3724,7 +3747,7 @@ SpinboxWidgetObjCmd(
 	break;
 
     case SB_CMD_DELETE: {
-	int first, last;
+	int first, last, code;
 
 	if ((objc < 3) || (objc > 4)) {
 	    Tcl_WrongNumArgs(interp, 2, objv, "firstIndex ?lastIndex?");
@@ -3743,7 +3766,10 @@ SpinboxWidgetObjCmd(
 	    }
 	}
 	if ((last >= first) && (entryPtr->state == STATE_NORMAL)) {
-	    DeleteChars(entryPtr, first, last - first);
+	    code = DeleteChars(entryPtr, first, last - first);
+            if (code != TCL_OK) {
+                goto error;
+            }
 	}
 	break;
     }
@@ -3803,7 +3829,7 @@ SpinboxWidgetObjCmd(
     }
 
     case SB_CMD_INSERT: {
-	int index;
+	int index, code;
 
 	if (objc != 4) {
 	    Tcl_WrongNumArgs(interp, 2, objv, "index text");
@@ -3814,7 +3840,10 @@ SpinboxWidgetObjCmd(
 	    goto error;
 	}
 	if (entryPtr->state == STATE_NORMAL) {
-	    InsertChars(entryPtr, index, Tcl_GetString(objv[3]));
+	    code = InsertChars(entryPtr, index, Tcl_GetString(objv[3]));
+            if (code != TCL_OK) {
+                goto error;
+            }
 	}
 	break;
     }
@@ -4024,16 +4053,22 @@ SpinboxWidgetObjCmd(
 	break;
     }
 
-    case SB_CMD_SET:
+    case SB_CMD_SET: {
+	int code = TCL_OK;
+
 	if (objc > 3) {
 	    Tcl_WrongNumArgs(interp, 2, objv, "?string?");
 	    goto error;
 	}
 	if (objc == 3) {
-	    EntryValueChanged(entryPtr, Tcl_GetString(objv[2]));
+	    code = EntryValueChanged(entryPtr, Tcl_GetString(objv[2]));
+            if (code != TCL_OK) {
+                goto error;
+            }
 	}
 	Tcl_SetObjResult(interp, Tcl_NewStringObj(entryPtr->string, -1));
 	break;
+    }
 
     case SB_CMD_VALIDATE: {
 	int code;
@@ -4174,7 +4209,7 @@ GetSpinboxElement(
  *	TCL_OK.
  *
  * Side effects:
- *	An background error condition may arise when invoking the callback.
+ *	A background error condition may arise when invoking the callback.
  *	The widget value may change.
  *
  *--------------------------------------------------------------
@@ -4205,6 +4240,7 @@ SpinboxInvoke(
 	return TCL_OK;
     }
 
+    code = TCL_OK;
     if (fabs(sbPtr->increment) > MIN_DBL_VAL) {
 	if (sbPtr->listObj != NULL) {
 	    Tcl_Obj *objPtr;
@@ -4250,7 +4286,7 @@ SpinboxInvoke(
 		}
 	    }
 	    Tcl_ListObjIndex(interp, sbPtr->listObj, sbPtr->eIndex, &objPtr);
-	    EntryValueChanged(entryPtr, Tcl_GetString(objPtr));
+	    code = EntryValueChanged(entryPtr, Tcl_GetString(objPtr));
 	} else if (!DOUBLES_EQ(sbPtr->fromValue, sbPtr->toValue)) {
 	    double dvalue;
 
@@ -4297,9 +4333,12 @@ SpinboxInvoke(
 		}
 	    }
 	    sprintf(sbPtr->formatBuf, sbPtr->valueFormat, dvalue);
-	    EntryValueChanged(entryPtr, sbPtr->formatBuf);
+	    code = EntryValueChanged(entryPtr, sbPtr->formatBuf);
 	}
     }
+    if (code != TCL_OK) {
+        return TCL_ERROR;
+    }
 
     if (sbPtr->command != NULL) {
 	Tcl_DStringInit(&script);
diff --git a/tests/entry.test b/tests/entry.test
index f261f0d..4cc9218 100644
--- a/tests/entry.test
+++ b/tests/entry.test
@@ -3460,8 +3460,6 @@ test entry-22.1 {lost namespaced textvar} -body {
     namespace eval test { variable foo {a b} }
     entry .e -textvariable ::test::foo
     namespace delete test
-    .e insert end "more stuff"
-    .e delete 5 end
     set ::test::foo
 } -cleanup {
     destroy .e
@@ -3470,13 +3468,30 @@ test entry-22.2 {lost namespaced textvar} -body {
     namespace eval test { variable foo {a b} }
     entry .e -textvariable ::test::foo
     namespace delete test
-    .e insert end "more stuff"
-    .e delete 5 end 
-    catch {set ::test::foo}
-    list [.e get] [.e cget -textvar] 
+    catch {.e insert end "more stuff"} result1
+    catch {.e delete 5 end } result2
+    catch {set ::test::foo} result3
+    list [.e get] [.e cget -textvar] $result1 $result2 $result3
 } -cleanup {
     destroy .e
-} -result [list "a bmo" ::test::foo]
+} -result [list "a bmo" ::test::foo \
+	{can't set "::test::foo": parent namespace doesn't exist} \
+	{can't set "::test::foo": parent namespace doesn't exist} \
+	{can't read "::test::foo": no such variable}]
+
+test entry-23.1 {error in trace proc attached to the textvariable} -setup {
+    destroy .e
+} -body {
+    trace variable myvar w traceit
+    proc traceit args {error "Intentional error here!"}
+    entry .e -textvariable myvar
+    catch {.e insert end mystring} result1
+    catch {.e delete 0} result2
+    list $result1 $result2
+} -cleanup {
+    destroy .e
+} -result [list {can't set "myvar": Intentional error here!} \
+    {can't set "myvar": Intentional error here!}]
 
 # Gathered comments about lacks
 # XXX Still need to write tests for EntryBlinkProc, EntryFocusProc,
diff --git a/tests/spinbox.test b/tests/spinbox.test
index b8170c5..f101640 100644
--- a/tests/spinbox.test
+++ b/tests/spinbox.test
@@ -3790,6 +3790,23 @@ test spinbox-23.1 {selection present while disabled, bug 637828} -body {
     destroy .e
 } -result {1 1 345}
 
+test spinbox-24.1 {error in trace proc attached to the textvariable} -setup {
+    destroy .s
+} -body {
+    trace variable myvar w traceit
+    proc traceit args {error "Intentional error here!"}
+    spinbox .s -textvariable myvar -from 1 -to 10
+    catch {.s set mystring} result1
+    catch {.s insert 0 mystring} result2
+    catch {.s delete 0} result3
+    catch {.s invoke buttonup} result4
+    list $result1 $result2 $result3 $result4
+} -cleanup {
+    destroy .s
+} -result [list {can't set "myvar": Intentional error here!} \
+    {can't set "myvar": Intentional error here!} \
+    {can't set "myvar": Intentional error here!} \
+    {can't set "myvar": Intentional error here!}]
 
 # Collected comments about lacks from the test
 # XXX Still need to write tests for SpinboxBlinkProc, SpinboxFocusProc,