Merge 576c8c09f7 into e42ed6808e

core/modules/editor/factory.js

@@ -48,8 +48,8 @@ function editTextWidgetFactory(toolbarEngine,nonToolbarEngine) {
 			this.toolbarNode = this.document.createElement("div");
 			this.toolbarNode.className = "tc-editor-toolbar";
 			parent.insertBefore(this.toolbarNode,nextSibling);
-			this.renderChildren(this.toolbarNode,null);
 			this.domNodes.push(this.toolbarNode);
+			this.renderChildren(this.toolbarNode,null);
 		}
 		// Create our element
 		var editInfo = this.getEditInfo(),

core/modules/parsers/wikiparser/rules/transcludeblock.js

@@ -23,27 +23,6 @@ exports.init = function(parser) {
 	this.matchRegExp = /\{\{([^\{\}\|]*)(?:\|\|([^\|\{\}]+))?(?:\|([^\{\}]+))?\}\}(?:\r?\n|$)/mg;
 };
 
-/*
-Reject the match if we don't have a template or text reference
-*/
-exports.findNextMatch = function(startPos) {
-	this.matchRegExp.lastIndex = startPos;
-	this.match = this.matchRegExp.exec(this.parser.source);
-	if(this.match) {
-		var template = $tw.utils.trim(this.match[2]),
-			textRef = $tw.utils.trim(this.match[1]);
-		// Bail if we don't have a template or text reference
-		if(!template && !textRef) {
-			return undefined;
-		} else {
-			return this.match.index;
-		}
-	} else {
-		return undefined;
-	}
-	return this.match ? this.match.index : undefined;
-};
-
 exports.parse = function() {
 	// Move past the match
 	this.parser.pos = this.matchRegExp.lastIndex;

core/modules/parsers/wikiparser/rules/transcludeinline.js

@@ -23,27 +23,6 @@ exports.init = function(parser) {
 	this.matchRegExp = /\{\{([^\{\}\|]*)(?:\|\|([^\|\{\}]+))?(?:\|([^\{\}]+))?\}\}/mg;
 };
 
-/*
-Reject the match if we don't have a template or text reference
-*/
-exports.findNextMatch = function(startPos) {
-	this.matchRegExp.lastIndex = startPos;
-	this.match = this.matchRegExp.exec(this.parser.source);
-	if(this.match) {
-		var template = $tw.utils.trim(this.match[2]),
-			textRef = $tw.utils.trim(this.match[1]);
-		// Bail if we don't have a template or text reference
-		if(!template && !textRef) {
-			return undefined;
-		} else {
-			return this.match.index;
-		}
-	} else {
-		return undefined;
-	}
-	return this.match ? this.match.index : undefined;
-};
-
 exports.parse = function() {
 	// Move past the match
 	this.parser.pos = this.matchRegExp.lastIndex;

core/modules/utils/errors.js

@@ -6,6 +6,7 @@ module-type: utils
 Custom errors for TiddlyWiki.
 
 \*/
+
 function TranscludeRecursionError() {
 	Error.apply(this,arguments);
 	this.signatures = Object.create(null);

core/modules/widgets/browse.js

@@ -80,8 +80,8 @@ BrowseWidget.prototype.render = function(parent,nextSibling) {
 	});
 	// Insert element
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 /*

core/modules/widgets/button.js

@@ -135,8 +135,8 @@ ButtonWidget.prototype.render = function(parent,nextSibling) {
 	}
 	// Insert element
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 /*

core/modules/widgets/checkbox.js

@@ -64,8 +64,8 @@ CheckboxWidget.prototype.render = function(parent,nextSibling) {
 	]);
 	// Insert the label into the DOM and render any children
 	parent.insertBefore(this.labelDomNode,nextSibling);
-	this.renderChildren(this.spanDomNode,null);
 	this.domNodes.push(this.labelDomNode);
+	this.renderChildren(this.spanDomNode,null);
 };
 
 CheckboxWidget.prototype.getValue = function() {

core/modules/widgets/diff-text.js

@@ -59,6 +59,8 @@ DiffTextWidget.prototype.render = function(parent,nextSibling) {
 	var domContainer = this.document.createElement("div"), 
 		domDiff = this.createDiffDom(diffs);
 	parent.insertBefore(domContainer,nextSibling);
+	// Save our container
+	this.domNodes.push(domContainer);
 	// Set variables
 	this.setVariable("diff-count",diffs.reduce(function(acc,diff) {
 		if(diff[0] !== dmp.DIFF_EQUAL) {
@@ -70,8 +72,6 @@ DiffTextWidget.prototype.render = function(parent,nextSibling) {
 	this.renderChildren(domContainer,null);
 	// Render the diff
 	domContainer.appendChild(domDiff);
-	// Save our container
-	this.domNodes.push(domContainer);
 };
 
 /*

core/modules/widgets/draggable.js

@@ -56,6 +56,7 @@ DraggableWidget.prototype.render = function(parent,nextSibling) {
 	});
 	// Insert the node into the DOM and render any children
 	parent.insertBefore(domNode,nextSibling);
+	this.domNodes.push(domNode);
 	this.renderChildren(domNode,null);
 	// Add event handlers
 	if(this.dragEnable) {
@@ -70,7 +71,6 @@ DraggableWidget.prototype.render = function(parent,nextSibling) {
 			selector: self.dragHandleSelector
 		});
 	}
-	this.domNodes.push(domNode);
 };
 
 /*

core/modules/widgets/droppable.js

@@ -57,8 +57,8 @@ DroppableWidget.prototype.render = function(parent,nextSibling) {
 	}
 	// Insert element
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 	// Stack of outstanding enter/leave events
 	this.currentlyEntered = [];
 };

core/modules/widgets/element.js

@@ -77,8 +77,8 @@ ElementWidget.prototype.render = function(parent,nextSibling) {
 	// Allow hooks to manipulate the DOM node. Eg: Add debug info
 	$tw.hooks.invokeHook("th-dom-rendering-element", domNode, this);
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 /*

core/modules/widgets/eventcatcher.js

@@ -106,8 +106,8 @@ EventWidget.prototype.render = function(parent,nextSibling) {
 	});
 	// Insert element
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 /*

core/modules/widgets/keyboard.js

@@ -45,8 +45,8 @@ KeyboardWidget.prototype.render = function(parent,nextSibling) {
 	]);
 	// Insert element
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 KeyboardWidget.prototype.handleChangeEvent = function(event) {

core/modules/widgets/link.js

@@ -50,8 +50,8 @@ LinkWidget.prototype.render = function(parent,nextSibling) {
 			destPrefix: "aria-"
 		});
 		parent.insertBefore(domNode,nextSibling);
-		this.renderChildren(domNode,null);
 		this.domNodes.push(domNode);
+		this.renderChildren(domNode,null);
 	}
 };
 
@@ -157,8 +157,8 @@ LinkWidget.prototype.renderLink = function(parent,nextSibling) {
 	});
 	// Insert the link into the DOM and render any children
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 LinkWidget.prototype.handleClickEvent = function(event) {

core/modules/widgets/password.js

@@ -42,8 +42,8 @@ PasswordWidget.prototype.render = function(parent,nextSibling) {
 	]);
 	// Insert the label into the DOM and render any children
 	parent.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 };
 
 PasswordWidget.prototype.handleChangeEvent = function(event) {

core/modules/widgets/radio.js

@@ -59,8 +59,8 @@ RadioWidget.prototype.render = function(parent,nextSibling) {
 	]);
 	// Insert the label into the DOM and render any children
 	parent.insertBefore(this.labelDomNode,nextSibling);
-	this.renderChildren(this.spanDomNode,null);
 	this.domNodes.push(this.labelDomNode);
+	this.renderChildren(this.spanDomNode,null);
 };
 
 RadioWidget.prototype.getValue = function() {

core/modules/widgets/reveal.js

@@ -40,6 +40,7 @@ RevealWidget.prototype.render = function(parent,nextSibling) {
 		domNode.setAttribute("style",this.style);
 	}
 	parent.insertBefore(domNode,nextSibling);
+	this.domNodes.push(domNode);
 	this.renderChildren(domNode,null);
 	if(!domNode.isTiddlyWikiFakeDom && this.type === "popup" && this.isOpen) {
 		this.positionPopup(domNode);
@@ -48,7 +49,6 @@ RevealWidget.prototype.render = function(parent,nextSibling) {
 	if(!this.isOpen) {
 		domNode.setAttribute("hidden","true");
 	}
-	this.domNodes.push(domNode);
 };
 
 RevealWidget.prototype.positionPopup = function(domNode) {

core/modules/widgets/scrollable.js

@@ -168,8 +168,8 @@ ScrollableWidget.prototype.render = function(parent,nextSibling) {
 	this.outerDomNode.className = this["class"] || "";
 	// Insert element
 	parent.insertBefore(this.outerDomNode,nextSibling);
-	this.renderChildren(this.innerDomNode,null);
 	this.domNodes.push(this.outerDomNode);
+	this.renderChildren(this.innerDomNode,null);
 	// If the scroll position is bound to a tiddler
 	if(this.scrollableBind) {
 		// After a delay for rendering, scroll to the bound position

core/modules/widgets/select.js

@@ -63,8 +63,8 @@ SelectWidget.prototype.render = function(parent,nextSibling) {
 		domNode.setAttribute("title",this.selectTooltip);
 	}
 	this.parentDomNode.insertBefore(domNode,nextSibling);
-	this.renderChildren(domNode,null);
 	this.domNodes.push(domNode);
+	this.renderChildren(domNode,null);
 	this.setSelectValue();
 	if(this.selectFocus == "yes") {
 		this.getSelectDomNode().focus();

core/modules/widgets/transclude.js

@@ -32,16 +32,26 @@ TranscludeWidget.prototype.render = function(parent,nextSibling) {
 	} catch(error) {
 		if(error instanceof $tw.utils.TranscludeRecursionError) {
 			// We were infinite looping.
-			// We need to try and abort as much of the loop as we can, so we will keep "throwing" upward until we find a transclusion that has a different signature.
-			// Hopefully that will land us just outside where the loop began. That's where we want to issue an error.
-			// Rendering widgets beneath this point may result in a freezing browser if they explode exponentially.
+			// We need to try and abort as much of the loop as we
+			// can, so we will keep "throwing" upward until we find
+			// a transclusion that has a different signature.
+			// Hopefully that will land us just outside where the
+			// loop began. That's where we want to issue an error.
+			// Rendering widgets beneath this point may result in a
+			// freezing browser if they explode exponentially.
 			var transcludeSignature = this.getVariable("transclusion");
 			if(this.getAncestorCount() > $tw.utils.TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH - 50) {
-				// For the first fifty transcludes we climb up, we simply collect signatures.
-				// We're assuming that those first 50 will likely include all transcludes involved in the loop.
+				// For the first fifty transcludes we climb up,
+				// we simply collect signatures.
+				// We're assuming those first 50 will likely
+				// include all transcludes involved in the loop.
 				error.signatures[transcludeSignature] = true;
 			} else if(!error.signatures[transcludeSignature]) {
-				// Now that we're past the first 50, let's look for the first signature that wasn't in the loop. That'll be where we print the error and resume rendering.
+				// Now that we're past the first 50, look for
+				// the first signature that wasn't in that loop.
+				// That's where we print the error and resume
+				// rendering.
+				this.removeChildDomNodes();
 				this.children = [this.makeChildWidget({type: "error", attributes: {
 					"$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")}
 				}})];

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

@@ -177,6 +177,29 @@ describe("Widget module", function() {
 		expect(wrapper.innerHTML).toBe("<span class=\"tc-error\">Recursive transclusion error in transclude widget</span> <span class=\"tc-error\">Recursive transclusion error in transclude widget</span>");
 	});
 
+	$tw.utils.each(["div","$button","$checkbox","$diff-text","$draggable","$droppable","dropzone","$eventcatcher","$keyboard","$link","$list filter=x variable=x","$radio","$reveal type=nomatch","$scrollable","$select","$view field=x"],function(tag) {
+		it(`${tag} cleans itself up if children rendering fails`, function() {
+			var wiki = new $tw.Wiki();
+			wiki.addTiddler({title: "TiddlerOne", text: `<$tiddler tiddler='TiddlerOne'><${tag}><$transclude />`});
+			var parseTreeNode = {type: "widget", children: [
+				{type: "transclude", attributes: {
+					"tiddler": {type: "string", value: "TiddlerOne"}
+				}}
+			]};
+			// Construct the widget node
+			var widgetNode = createWidgetNode(parseTreeNode,wiki);
+			// Render the widget node to the DOM
+			var wrapper = renderWidgetNode(widgetNode);
+			// We don't actually care exactly what the HTML contains,
+			// only that it's reasonably sized. If it's super large,
+			// that means the widget containing the bad transclusion
+			// didn't figure out how to clean itself up, and it cloned a bunch.
+			var html = wrapper.innerHTML;
+			expect(html).toContain("Recursive transclusion error in transclude widget");
+			expect(html.length).toBeLessThan(256, "CONTENTS: " + html);
+		});
+	});
+
 	it("should handle many-tiddler recursion with branching nodes", function() {
 		var wiki = new $tw.Wiki();
 		// Add a tiddler

editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid

@@ -0,0 +1,10 @@
+title: $:/changenotes/5.4.0/#9548
+description: Better infinite transclude recursion handling
+release: 5.4.0
+tags: $:/tags/ChangeNote
+change-type: bugfix
+change-category: widget
+github-links: https://github.com/TiddlyWiki/TiddlyWiki5/pull/9458
+github-contributors: Flibbles
+
+Fixed issue where exceptions occurring during widget rendering could result in junk DOM nodes remaining in widget tree. This was very obvious when max recursion depth exceptions occurred.