From 2385bd978fd6c12ea651ca678220681d35323b76 Mon Sep 17 00:00:00 2001
From: Cameron Fischer <fischer.cameron@gmail.com>
Date: Thu, 23 Apr 2020 04:10:52 -0400
Subject: [PATCH] Fix importvariables crash (#4593)

* Fixed issue: multi nonMacro imports broke everything

* Fixed issue: dead variables in import might linger
---
 core/modules/widgets/importvariables.js     | 11 ++++-
 editions/test/tiddlers/tests/test-widget.js | 49 +++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/core/modules/widgets/importvariables.js b/core/modules/widgets/importvariables.js
index 3ea1d1174..56cc0852c 100644
--- a/core/modules/widgets/importvariables.js
+++ b/core/modules/widgets/importvariables.js
@@ -38,6 +38,8 @@ Compute the internal state of the widget
 */
 ImportVariablesWidget.prototype.execute = function(tiddlerList) {
 	var widgetPointer = this;
+	// Got to flush all the accumulated variables
+	this.variables = new this.variablesConstructor();
 	// Get our parameters
 	this.filter = this.getAttribute("filter");
 	// Compute the filter
@@ -70,7 +72,14 @@ ImportVariablesWidget.prototype.execute = function(tiddlerList) {
 						widgetPointer.variables[key] = widget.variables[key];
 					});
 				} else {
-					widgetPointer.makeChildWidgets([node]);
+					widgetPointer.children = [widgetPointer.makeChildWidget(node)];
+					// No more regenerating children for
+					// this widget. If it needs to refresh,
+					// it'll do so along with the the whole
+					// importvariable tree.
+					if (widgetPointer != this) {
+						widgetPointer.makeChildWidgets = function(){};
+					}
 					widgetPointer = widgetPointer.children[0];
 				}
 				parseTreeNode = parseTreeNode.children && parseTreeNode.children[0];
diff --git a/editions/test/tiddlers/tests/test-widget.js b/editions/test/tiddlers/tests/test-widget.js
index c20305746..0efc8b126 100755
--- a/editions/test/tiddlers/tests/test-widget.js
+++ b/editions/test/tiddlers/tests/test-widget.js
@@ -504,6 +504,55 @@ describe("Widget module", function() {
 		expect(wrapper.innerHTML).toBe("<p>Aval Bval Cval</p>");
 	});
 
+	it("can have more than one macroDef variable imported", function() {
+		var wiki = new $tw.Wiki();
+		wiki.addTiddlers([
+			{title: "ABC", text: "<$set name=A value=A>\n\n<$set name=B value=B>\n\n<$set name=C value=C>\n\ndummy text</$set></$set></$set>"},
+			{title: "D", text: "\\define D() D"}]);
+		// A and B shouldn't chew up C just cause it's a macroDef
+		var text = "\\import ABC D\n<<A>> <<B>> <<C>> <<D>>";
+		var widgetNode = createWidgetNode(parseText(text,wiki),wiki);
+		// Render the widget node to the DOM
+		var wrapper = renderWidgetNode(widgetNode);
+		// Test the rendering
+		expect(wrapper.innerHTML).toBe("<p>A B C D</p>");
+	});
+
+	it("import doesn't hold onto dead variables", function() {
+		var wiki = new  $tw.Wiki();
+		wiki.addTiddlers([
+			{title: "ABC", text: "\\define A() A\n\\define B() B\n<$set name=C value=C>\n\n</$set>"},
+			{title: "DE", text: "\\define D() D\n\\define E() E"}]);
+		var text = "\\import ABC DE\ncontent";
+		var widgetNode = createWidgetNode(parseText(text,wiki),wiki);
+		// Render the widget node to the DOM
+		renderWidgetNode(widgetNode);
+		var childNode = widgetNode;
+		while (childNode.children.length > 0) {
+			childNode = childNode.children[0];
+		}
+		// First make sure A and B imported
+		expect(childNode.getVariable("A")).toBe("A");
+		expect(childNode.getVariable("B")).toBe("B");
+		expect(childNode.getVariable("C")).toBe("C");
+		expect(childNode.getVariable("D")).toBe("D");
+		expect(childNode.getVariable("E")).toBe("E");
+		// Then change A and remove B
+		wiki.addTiddler({title: "ABC", text: "\\define A() A2\n<$set name=C value=C2>\n\n</$set>"});
+		wiki.addTiddler({title: "DE", text: "\\define D() D2"});
+		widgetNode.refresh({"ABC": {modified: true}, "DE": {modified: true}});
+		var childNode = widgetNode;
+		while (childNode.children.length > 0) {
+			childNode = childNode.children[0];
+		}
+		// Make sure \import recognized changes and deletions
+		expect(childNode.getVariable("A")).toBe("A2");
+		expect(childNode.getVariable("B")).toBe(undefined);
+		expect(childNode.getVariable("C")).toBe("C2");
+		expect(childNode.getVariable("D")).toBe("D2");
+		expect(childNode.getVariable("E")).toBe(undefined);
+	});
+
 	/** Special case. <$importvariables> has different handling if
 	 *  it doesn't end up importing any variables. Make sure it
 	 *  doesn't forget its childrenNodes.