Detect recursion by tracking widget tree depth

The old recursion marker approach was very slow, and didn't catch test cases like editions/test/tiddlers/tests/data/transclude/Recursion.tid

core/modules/widgets/error.js

@@ -0,0 +1,63 @@
+/*\
+title: $:/core/modules/widgets/error.js
+type: application/javascript
+module-type: widget
+
+Error widget
+
+\*/
+(function(){
+
+/*jslint node: true, browser: true */
+/*global $tw: false */
+"use strict";
+
+var Widget = require("$:/core/modules/widgets/widget.js").widget;
+
+var ErrorWidget = function(parseTreeNode,options) {
+	this.initialise(parseTreeNode,options);
+};
+
+/*
+Inherit from the base widget class
+*/
+ErrorWidget.prototype = new Widget();
+
+/*
+Render this widget into the DOM
+*/
+ErrorWidget.prototype.render = function(parent,nextSibling) {
+	this.parentDomNode = parent;
+	this.computeAttributes();
+	this.execute();
+	var message = this.getAttribute("$message","Unknown error"),
+		domNode = this.document.createElement("span");
+	domNode.appendChild(this.document.createTextNode(message));
+	domNode.className = "tc-error";
+	parent.insertBefore(domNode,nextSibling);
+	this.domNodes.push(domNode);
+};
+
+/*
+Compute the internal state of the widget
+*/
+ErrorWidget.prototype.execute = function() {
+	// Nothing to do for a text node
+};
+
+/*
+Selectively refreshes the widget if needed. Returns true if the widget or any of its children needed re-rendering
+*/
+ErrorWidget.prototype.refresh = function(changedTiddlers) {
+	var changedAttributes = this.computeAttributes();
+	if(changedAttributes["$message"]) {
+		this.refreshSelf();
+		return true;
+	} else {
+		return false;
+	}
+};
+
+exports.error = ErrorWidget;
+
+})();

core/modules/widgets/transclude.js

@@ -62,25 +62,10 @@ TranscludeWidget.prototype.execute = function() {
 			parseTreeNodes = [{type: "text", text: plainText}];
 			break;
 	}
-	// Set context variables for recursion detection
-	var recursionMarker = this.makeRecursionMarker();
-	if(this.recursionMarker === "yes") {
-		this.setVariable("$transclusion",recursionMarker);
-	}
 	// Set the legacy transclusion context variables only if we're not transcluding a variable
 	if(!this.transcludeVariable) {
-		var legacyRecursionMarker = this.makeLegacyRecursionMarker();
-		this.setVariable("transclusion",legacyRecursionMarker);	
-	}
-	// Check for recursion
-	if(target.parser) {
-		if(this.parentWidget && this.parentWidget.hasVariable("$transclusion",recursionMarker)) {
-			parseTreeNodes = [{type: "element", tag: "span", attributes: {
-				"class": {type: "string", value: "tc-error"}
-			}, children: [
-				{type: "text", text: $tw.language.getString("Error/RecursiveTransclusion")}
-			]}];
-		}
+		var recursionMarker = this.makeRecursionMarker();
+		this.setVariable("transclusion",recursionMarker);	
 	}
 	// Construct the child widgets
 	this.makeChildWidgets(parseTreeNodes);
@@ -335,29 +320,10 @@ TranscludeWidget.prototype.getTransclusionSlotValue = function(name,defaultParse
 	}
 };
 
-/*
-Compose a string comprising the attributes and variables to identify this transclusion for recursion detection
-*/
-TranscludeWidget.prototype.makeRecursionMarker = function() {
-	var marker = {
-		attributes: {},
-		variables: {}
-	}
-	$tw.utils.each(this.attributes,function(value,name) {
-		marker.attributes[name] = value;
-	});
-	for(var name in this.variables) {
-		if(name !== "$transclusion") {
-			marker.variables[name] = this.getVariable(name);
-		}
-	};
-	return JSON.stringify(marker);
-};
-
 /*
 Compose a string comprising the title, field and/or index to identify this transclusion for recursion detection
 */
-TranscludeWidget.prototype.makeLegacyRecursionMarker = function() {
+TranscludeWidget.prototype.makeRecursionMarker = function() {
 	var output = [];
 	output.push("{");
 	output.push(this.getVariable("currentTiddler",{defaultValue: ""}));

core/modules/widgets/widget.js

@@ -12,6 +12,9 @@ Widget base class
 /*global $tw: false */
 "use strict";
 
+/* Maximum permitted depth of the widget tree for recursion detection */
+var MAX_WIDGET_TREE_DEPTH = 1000;
+
 /*
 Create a widget object for a parse tree node
 	parseTreeNode: reference to the parse tree node to be rendered
@@ -375,6 +378,20 @@ Widget.prototype.assignAttributes = function(domNode,options) {
 	}
 };
 
+/*
+Get the number of ancestor widgets for this widget
+*/
+Widget.prototype.getAncestorCount = function() {
+	if(this.ancestorCount === undefined) {
+		if(this.parentWidget) {
+			this.ancestorCount = this.parentWidget.getAncestorCount() + 1;
+		} else {
+			this.ancestorCount = 0;
+		}
+	}
+	return this.ancestorCount;
+};
+
 /*
 Make child widgets correspondng to specified parseTreeNodes
 */
@@ -382,21 +399,30 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) {
 	options = options || {};
 	this.children = [];
 	var self = this;
-	// Create set variable widgets for each variable
-	$tw.utils.each(options.variables,function(value,name) {
-		var setVariableWidget = {
-			type: "set",
-			attributes: {
-				name: {type: "string", value: name},
-				value: {type: "string", value: value}
-			},
-			children: parseTreeNodes
-		};
-		parseTreeNodes = [setVariableWidget];
-	});
-	$tw.utils.each(parseTreeNodes || (this.parseTreeNode && this.parseTreeNode.children),function(childNode) {
-		self.children.push(self.makeChildWidget(childNode));
-	});
+	// Check for too much recursion
+	if(this.getAncestorCount() > MAX_WIDGET_TREE_DEPTH) {
+		// Error message needs special permission not to cause a recursive error loop
+		this.children.push(this.makeChildWidget({type: "error", attributes: {
+			"$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")}
+		}}));
+	} else {
+		// Create set variable widgets for each variable
+		$tw.utils.each(options.variables,function(value,name) {
+			var setVariableWidget = {
+				type: "set",
+				attributes: {
+					name: {type: "string", value: name},
+					value: {type: "string", value: value}
+				},
+				children: parseTreeNodes
+			};
+			parseTreeNodes = [setVariableWidget];
+		});
+		// Create the child widgets
+		$tw.utils.each(parseTreeNodes || (this.parseTreeNode && this.parseTreeNode.children),function(childNode) {
+			self.children.push(self.makeChildWidget(childNode));
+		});
+	}
 };
 
 /*

editions/test/tiddlers/tests/data/transclude/Recursion.tid

@@ -0,0 +1,17 @@
+title: Transclude/Recursion
+description: Transclusion recursion detection
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\whitespace trim
+\procedure recurse(a:0)
+<$transclude $variable="recurse" a={{{ [<a>add[1]] }}}/>
+\end
+
+<<recurse>>
++
+title: ExpectedResult
+
+<p><span class="tc-error">Recursive transclusion error in transclude widget</span></p>

editions/test/tiddlers/tests/test-widget.js

@@ -143,7 +143,7 @@ describe("Widget module", function() {
 		var wiki = new $tw.Wiki();
 		// Add a tiddler
 		wiki.addTiddlers([
-			{title: "TiddlerOne", text: "<$transclude tiddler='TiddlerTwo'/>\n"},
+			{title: "TiddlerOne", text: "<$transclude tiddler='TiddlerTwo'/>"},
 			{title: "TiddlerTwo", text: "<$transclude tiddler='TiddlerOne'/>"}
 		]);
 		// Test parse tree
@@ -157,7 +157,7 @@ describe("Widget module", function() {
 		// Render the widget node to the DOM
 		var wrapper = renderWidgetNode(widgetNode);
 		// Test the rendering
-		expect(wrapper.innerHTML).toBe("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span>\n\n");
+		expect(wrapper.innerHTML).toBe("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span>");
 	});
 
 	it("should deal with SVG elements", function() {