From bc89805368119ca191026424a6970da29f89d168 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Sun, 10 Dec 2023 23:08:02 -0500 Subject: [PATCH 01/11] Introduced preliminary idea for infinite recurse exception --- core/modules/utils/errors.js | 17 +++++++++++++++++ core/modules/widgets/transclude.js | 19 ++++++++++++++++++- core/modules/widgets/widget.js | 4 +--- editions/test/tiddlers/tests/test-widget.js | 20 ++++++++++++++++++++ 4 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 core/modules/utils/errors.js diff --git a/core/modules/utils/errors.js b/core/modules/utils/errors.js new file mode 100644 index 000000000..1719aa60a --- /dev/null +++ b/core/modules/utils/errors.js @@ -0,0 +1,17 @@ +/*\ +title: $:/core/modules/utils/errors.js +type: application/javascript +module-type: utils + +Custom errors for TiddlyWiki. + +\*/ +(function(){ + +function TranscludeRecursionError(transcludeMarker) { + this.marker = transcludeMarker; +}; + +exports.TranscludeRecursionError = TranscludeRecursionError; + +})(); diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index d30ab1fa7..7702641be 100755 --- a/core/modules/widgets/transclude.js +++ b/core/modules/widgets/transclude.js @@ -30,7 +30,24 @@ TranscludeWidget.prototype.render = function(parent,nextSibling) { this.parentDomNode = parent; this.computeAttributes(); this.execute(); - this.renderChildren(parent,nextSibling); + try { + this.renderChildren(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. + if(error.marker !== this.getVariable("transclusion")) { + this.children = [this.makeChildWidget({type: "error", attributes: { + "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} + }})]; + this.renderChildren(parent,nextSibling); + return; + } + } + throw error; + } }; /* diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index af4892b9e..f45e608a8 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -495,9 +495,7 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) { var self = this; // Check for too much recursion if(this.getAncestorCount() > MAX_WIDGET_TREE_DEPTH) { - this.children.push(this.makeChildWidget({type: "error", attributes: { - "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} - }})); + throw new $tw.utils.TranscludeRecursionError(this.getVariable("transclusion")); } else { // Create set variable widgets for each variable $tw.utils.each(options.variables,function(value,name) { diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 0d1351f31..25db8aad7 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -160,6 +160,26 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget"); }); + it("should handle recursion with branching nodes", function() { + var wiki = new $tw.Wiki(); + // Add a tiddler + wiki.addTiddlers([ + {title: "TiddlerOne", text: "<$tiddler tiddler='TiddlerOne'><$transclude /> <$transclude />"}, + ]); + // Test parse tree + 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); + // Test the rendering + expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); + }); + it("should deal with SVG elements", function() { var wiki = new $tw.Wiki(); // Construct the widget node From 777176d7ec4dd93907d53770d771a6690fe0d3d0 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Mon, 11 Dec 2023 21:55:03 -0500 Subject: [PATCH 02/11] Better handling of infinite recursion But it could be better still... --- core/modules/utils/errors.js | 5 +++-- core/modules/widgets/transclude.js | 8 ++++++- core/modules/widgets/widget.js | 2 +- editions/test/tiddlers/tests/test-widget.js | 23 ++++++++++++++++++++- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/core/modules/utils/errors.js b/core/modules/utils/errors.js index 1719aa60a..73f60ae84 100644 --- a/core/modules/utils/errors.js +++ b/core/modules/utils/errors.js @@ -8,8 +8,9 @@ Custom errors for TiddlyWiki. \*/ (function(){ -function TranscludeRecursionError(transcludeMarker) { - this.marker = transcludeMarker; +function TranscludeRecursionError(depth) { + this.depth = depth; + this.signatures = Object.create(null); }; exports.TranscludeRecursionError = TranscludeRecursionError; diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index 7702641be..ce51e2102 100755 --- a/core/modules/widgets/transclude.js +++ b/core/modules/widgets/transclude.js @@ -38,7 +38,13 @@ TranscludeWidget.prototype.render = function(parent,nextSibling) { // 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. - if(error.marker !== this.getVariable("transclusion")) { + var transcludeSignature = this.getVariable("transclusion"); + if(this.getAncestorCount() > error.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. + 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. this.children = [this.makeChildWidget({type: "error", attributes: { "$message": {type: "string", value: $tw.language.getString("Error/RecursiveTransclusion")} }})]; diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index f45e608a8..1fe951787 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -495,7 +495,7 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) { var self = this; // Check for too much recursion if(this.getAncestorCount() > MAX_WIDGET_TREE_DEPTH) { - throw new $tw.utils.TranscludeRecursionError(this.getVariable("transclusion")); + throw new $tw.utils.TranscludeRecursionError(MAX_WIDGET_TREE_DEPTH); } else { // Create set variable widgets for each variable $tw.utils.each(options.variables,function(value,name) { diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 25db8aad7..9724c72b4 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -160,7 +160,7 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget"); }); - it("should handle recursion with branching nodes", function() { + it("should handle single-tiddler recursion with branching nodes", function() { var wiki = new $tw.Wiki(); // Add a tiddler wiki.addTiddlers([ @@ -180,6 +180,27 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); }); + fit("should handle many-tiddler recursion with branching nodes", function() { + var wiki = new $tw.Wiki(); + // Add a tiddler + wiki.addTiddlers([ + {title: "TiddlerOne", text: "<$transclude tiddler='TiddlerTwo'/> <$transclude tiddler='TiddlerTwo'/>"}, + {title: "TiddlerTwo", text: "<$transclude tiddler='TiddlerOne'/>"} + ]); + // Test parse tree + 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); + // Test the rendering + expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget"); + }); + it("should deal with SVG elements", function() { var wiki = new $tw.Wiki(); // Construct the widget node From 19a39f231cd13117db78f92bf3eb4c3029cc9e41 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Mon, 11 Dec 2023 22:14:24 -0500 Subject: [PATCH 03/11] the TransclusionError is a proper error Moved the magic number to be on the error's class. Not sure if that's a great idea. --- core/modules/utils/errors.js | 9 +++++++-- core/modules/widgets/transclude.js | 2 +- core/modules/widgets/widget.js | 7 ++----- editions/test/tiddlers/tests/test-widget.js | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/core/modules/utils/errors.js b/core/modules/utils/errors.js index 73f60ae84..fac4b3fa7 100644 --- a/core/modules/utils/errors.js +++ b/core/modules/utils/errors.js @@ -8,11 +8,16 @@ Custom errors for TiddlyWiki. \*/ (function(){ -function TranscludeRecursionError(depth) { - this.depth = depth; +function TranscludeRecursionError() { + Error.apply(this,arguments); this.signatures = Object.create(null); }; +/* Maximum permitted depth of the widget tree for recursion detection */ +TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH = 1000; + +TranscludeRecursionError.prototype = Object.create(Error); + exports.TranscludeRecursionError = TranscludeRecursionError; })(); diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index ce51e2102..35b4941bd 100755 --- a/core/modules/widgets/transclude.js +++ b/core/modules/widgets/transclude.js @@ -39,7 +39,7 @@ TranscludeWidget.prototype.render = function(parent,nextSibling) { // 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() > error.depth - 50) { + 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. error.signatures[transcludeSignature] = true; diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index 1fe951787..601ff68f9 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -12,9 +12,6 @@ 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 @@ -494,8 +491,8 @@ Widget.prototype.makeChildWidgets = function(parseTreeNodes,options) { this.children = []; var self = this; // Check for too much recursion - if(this.getAncestorCount() > MAX_WIDGET_TREE_DEPTH) { - throw new $tw.utils.TranscludeRecursionError(MAX_WIDGET_TREE_DEPTH); + if(this.getAncestorCount() > $tw.utils.TranscludeRecursionError.MAX_WIDGET_TREE_DEPTH) { + throw new $tw.utils.TranscludeRecursionError(); } else { // Create set variable widgets for each variable $tw.utils.each(options.variables,function(value,name) { diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 9724c72b4..1c7665a53 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -180,7 +180,7 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); }); - fit("should handle many-tiddler recursion with branching nodes", function() { + it("should handle many-tiddler recursion with branching nodes", function() { var wiki = new $tw.Wiki(); // Add a tiddler wiki.addTiddlers([ From 7196099f43c27369cdcf37284701a19bf303d592 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Mon, 11 Dec 2023 23:53:32 -0500 Subject: [PATCH 04/11] Fixed minor minor issue that came up in conflict The minor fix to the jasmine regexp that escaped a '+' somehow broke some random test. --- editions/test/tiddlers/tests/data/transclude/Recursion.tid | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/editions/test/tiddlers/tests/data/transclude/Recursion.tid b/editions/test/tiddlers/tests/data/transclude/Recursion.tid index d75e671eb..b834f3765 100644 --- a/editions/test/tiddlers/tests/data/transclude/Recursion.tid +++ b/editions/test/tiddlers/tests/data/transclude/Recursion.tid @@ -7,7 +7,8 @@ title: Output \whitespace trim <$transclude $tiddler="Output"/> + + title: ExpectedResult -

Recursive transclusion error in transclude widget

\ No newline at end of file +Recursive transclusion error in transclude widget \ No newline at end of file From baf7b8d7821d28d379649f256eaabb8ce3dc59ce Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 15:34:40 -0500 Subject: [PATCH 05/11] Removing patch fix for recursion errors --- .../wikiparser/rules/transcludeblock.js | 21 ------------------- .../wikiparser/rules/transcludeinline.js | 21 ------------------- 2 files changed, 42 deletions(-) diff --git a/core/modules/parsers/wikiparser/rules/transcludeblock.js b/core/modules/parsers/wikiparser/rules/transcludeblock.js index 5c5367cb4..525113d5d 100644 --- a/core/modules/parsers/wikiparser/rules/transcludeblock.js +++ b/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; diff --git a/core/modules/parsers/wikiparser/rules/transcludeinline.js b/core/modules/parsers/wikiparser/rules/transcludeinline.js index 57def3e33..4ae58e617 100644 --- a/core/modules/parsers/wikiparser/rules/transcludeinline.js +++ b/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; From d7ba1a1024bf6a26cf5c77577cf4dc390778f6b6 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 17:05:02 -0500 Subject: [PATCH 06/11] Fixed issue where buttton and other widgets don't clean up --- core/modules/widgets/button.js | 2 +- core/modules/widgets/checkbox.js | 2 +- core/modules/widgets/element.js | 2 +- core/modules/widgets/transclude.js | 22 ++++++++++++++------ editions/test/tiddlers/tests/test-widget.js | 23 +++++++++++++++++++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/core/modules/widgets/button.js b/core/modules/widgets/button.js index 8f6f14376..996d7f255 100644 --- a/core/modules/widgets/button.js +++ b/core/modules/widgets/button.js @@ -128,8 +128,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); }; /* diff --git a/core/modules/widgets/checkbox.js b/core/modules/widgets/checkbox.js index 7fe62fb42..f32fd7fd7 100644 --- a/core/modules/widgets/checkbox.js +++ b/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() { diff --git a/core/modules/widgets/element.js b/core/modules/widgets/element.js index 8b0a88e86..cb1f9d243 100755 --- a/core/modules/widgets/element.js +++ b/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); }; /* diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js index 9bec28b07..f1ed74e34 100755 --- a/core/modules/widgets/transclude.js +++ b/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")} }})]; diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 7d1cbc329..ca63f1dca 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -177,6 +177,29 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); }); + $tw.utils.each(["div","$button","$checkbox"],function(tag) { + it(`${tag} widget 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 From 5f722b1003765f9238d761b89b01ae11604f9fe4 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 17:12:07 -0500 Subject: [PATCH 07/11] Added release notes for #9548 --- editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid diff --git a/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid b/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid new file mode 100644 index 000000000..e8f1735a7 --- /dev/null +++ b/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 + +This introduces and attempt to be far more robust in detecting infinite recursion in transclusion, and to be better about cleaning up the widget tree and reporting it. From d8e61158b1380b3c7e554d6db501b6365fe76221 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 17:16:55 -0500 Subject: [PATCH 08/11] Update test-widget.js If I don't fix those indentations, the entire TW codebase will explode or soemthing. --- editions/test/tiddlers/tests/test-widget.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index ca63f1dca..e4ad9c6f6 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -182,10 +182,10 @@ describe("Widget module", 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"} - }} - ]}; + {type: "transclude", attributes: { + "tiddler": {type: "string", value: "TiddlerOne"} + }} + ]}; // Construct the widget node var widgetNode = createWidgetNode(parseTreeNode,wiki); // Render the widget node to the DOM @@ -195,7 +195,7 @@ describe("Widget module", function() { // 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).toContain("Recursive transclusion error in transclude widget"); expect(html.length).toBeLessThan(256, "CONTENTS: " + html); }); }); From fdc127223bfcdfa2454b5b7bf32d71bbd1b21be1 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 17:20:36 -0500 Subject: [PATCH 09/11] Update test-widget.js These lint problems are wasting my time. --- editions/test/tiddlers/tests/test-widget.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index e4ad9c6f6..24d732d8f 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -183,8 +183,8 @@ describe("Widget module", function() { wiki.addTiddler({title: "TiddlerOne", text: `<$tiddler tiddler='TiddlerOne'><${tag}><$transclude />`}); var parseTreeNode = {type: "widget", children: [ {type: "transclude", attributes: { - "tiddler": {type: "string", value: "TiddlerOne"} - }} + "tiddler": {type: "string", value: "TiddlerOne"} + }} ]}; // Construct the widget node var widgetNode = createWidgetNode(parseTreeNode,wiki); From 829d55aa10fdaacadfdaaed98ca929c35aeb3bb5 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 22:21:07 -0500 Subject: [PATCH 10/11] Fixed all core widgets to not leak when renderChildren fails --- core/modules/editor/factory.js | 2 +- core/modules/widgets/browse.js | 2 +- core/modules/widgets/diff-text.js | 4 ++-- core/modules/widgets/draggable.js | 2 +- core/modules/widgets/droppable.js | 2 +- core/modules/widgets/eventcatcher.js | 2 +- core/modules/widgets/keyboard.js | 2 +- core/modules/widgets/link.js | 4 ++-- core/modules/widgets/password.js | 2 +- core/modules/widgets/radio.js | 2 +- core/modules/widgets/reveal.js | 2 +- core/modules/widgets/scrollable.js | 2 +- core/modules/widgets/select.js | 2 +- editions/test/tiddlers/tests/test-widget.js | 4 ++-- 14 files changed, 17 insertions(+), 17 deletions(-) diff --git a/core/modules/editor/factory.js b/core/modules/editor/factory.js index 2fd323c0b..7ae54e227 100644 --- a/core/modules/editor/factory.js +++ b/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(), diff --git a/core/modules/widgets/browse.js b/core/modules/widgets/browse.js index b0173f6e2..47df1f3b7 100644 --- a/core/modules/widgets/browse.js +++ b/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); }; /* diff --git a/core/modules/widgets/diff-text.js b/core/modules/widgets/diff-text.js index 88175b17a..a9109716c 100644 --- a/core/modules/widgets/diff-text.js +++ b/core/modules/widgets/diff-text.js @@ -54,6 +54,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) { @@ -65,8 +67,6 @@ DiffTextWidget.prototype.render = function(parent,nextSibling) { this.renderChildren(domContainer,null); // Render the diff domContainer.appendChild(domDiff); - // Save our container - this.domNodes.push(domContainer); }; /* diff --git a/core/modules/widgets/draggable.js b/core/modules/widgets/draggable.js index 0bb2fc169..fbc935fb2 100644 --- a/core/modules/widgets/draggable.js +++ b/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); }; /* diff --git a/core/modules/widgets/droppable.js b/core/modules/widgets/droppable.js index 72f1a67f7..7f88ac223 100644 --- a/core/modules/widgets/droppable.js +++ b/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 = []; }; diff --git a/core/modules/widgets/eventcatcher.js b/core/modules/widgets/eventcatcher.js index 3268f6cab..2579858b1 100644 --- a/core/modules/widgets/eventcatcher.js +++ b/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); }; /* diff --git a/core/modules/widgets/keyboard.js b/core/modules/widgets/keyboard.js index bc452b9de..132ef9a18 100644 --- a/core/modules/widgets/keyboard.js +++ b/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) { diff --git a/core/modules/widgets/link.js b/core/modules/widgets/link.js index d2c599542..cb9225197 100755 --- a/core/modules/widgets/link.js +++ b/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) { diff --git a/core/modules/widgets/password.js b/core/modules/widgets/password.js index 59b084940..f429f881e 100644 --- a/core/modules/widgets/password.js +++ b/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) { diff --git a/core/modules/widgets/radio.js b/core/modules/widgets/radio.js index 4587df65f..33e4aa58e 100644 --- a/core/modules/widgets/radio.js +++ b/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() { diff --git a/core/modules/widgets/reveal.js b/core/modules/widgets/reveal.js index 5c2580f9e..e66b0fb79 100755 --- a/core/modules/widgets/reveal.js +++ b/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) { diff --git a/core/modules/widgets/scrollable.js b/core/modules/widgets/scrollable.js index 42705a61b..265c6f7d8 100644 --- a/core/modules/widgets/scrollable.js +++ b/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 diff --git a/core/modules/widgets/select.js b/core/modules/widgets/select.js index 8bc23542c..bef7b14c7 100644 --- a/core/modules/widgets/select.js +++ b/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(); diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js index 24d732d8f..9fa412626 100755 --- a/editions/test/tiddlers/tests/test-widget.js +++ b/editions/test/tiddlers/tests/test-widget.js @@ -177,8 +177,8 @@ describe("Widget module", function() { expect(wrapper.innerHTML).toBe("Recursive transclusion error in transclude widget Recursive transclusion error in transclude widget"); }); - $tw.utils.each(["div","$button","$checkbox"],function(tag) { - it(`${tag} widget cleans itself up if children rendering fails`, function() { + $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: [ From 576c8c09f78f82bf32acced28e5eb96e4681d822 Mon Sep 17 00:00:00 2001 From: Cameron Fischer Date: Wed, 7 Jan 2026 22:25:22 -0500 Subject: [PATCH 11/11] Updated release notes to reflect what I'm actually fixing --- editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid b/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid index e8f1735a7..50a2b6afd 100644 --- a/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid +++ b/editions/tw5.com/tiddlers/releasenotes/5.4.0/#9548.tid @@ -7,4 +7,4 @@ change-category: widget github-links: https://github.com/TiddlyWiki/TiddlyWiki5/pull/9458 github-contributors: Flibbles -This introduces and attempt to be far more robust in detecting infinite recursion in transclusion, and to be better about cleaning up the widget tree and reporting it. +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.