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/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; diff --git a/core/modules/utils/errors.js b/core/modules/utils/errors.js index 40e0a4f8f..b955e094f 100644 --- a/core/modules/utils/errors.js +++ b/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); 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/button.js b/core/modules/widgets/button.js index e946b7da9..0a5f955e3 100644 --- a/core/modules/widgets/button.js +++ b/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); }; /* 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/diff-text.js b/core/modules/widgets/diff-text.js index 5aa1344d6..d91d4b96a 100644 --- a/core/modules/widgets/diff-text.js +++ b/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); }; /* 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/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/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/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..9fa412626 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","$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 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..50a2b6afd --- /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 + +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.