From 51bdf60ee868cbf3943d086caa189c5d280b8f54 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Fri, 23 Sep 2022 00:52:55 +0700 Subject: [PATCH 1/7] Fix bug when using built-in `list` field as listField parameter to checkbox widget (#6897) * Fix bug with checkbox widget and `list` field The `list` field is stored as a list and frozen against modifications, and getFieldList() returns it directly without creating a copy. So before we modify it, we need to make a copy so we're not modifying a frozen list. This bug doesn't manifest with custom fields, which are stored as strings, only with the built-in `list` field. * Fix checkboxes referencing non-existent tiddlers This fixes the "tiddler is undefined" error when a checkbox's listField property references a tiddler that doesn't (yet) exist. * Better logic for checkbox listField handling If the field contains an array, then it's almost certainly referenced elsewhere and needs a defensive copy made. If it contained a string, then it's safe to modify without making a defensive copy. --- core/modules/widgets/checkbox.js | 17 ++++++--- .../tiddlers/tests/test-checkbox-widget.js | 36 ++++++++++++++++++- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/core/modules/widgets/checkbox.js b/core/modules/widgets/checkbox.js index a81ca837a..68ebe7980 100644 --- a/core/modules/widgets/checkbox.js +++ b/core/modules/widgets/checkbox.js @@ -66,14 +66,14 @@ CheckboxWidget.prototype.render = function(parent,nextSibling) { CheckboxWidget.prototype.getValue = function() { var tiddler = this.wiki.getTiddler(this.checkboxTitle); if(tiddler || this.checkboxFilter) { - if(this.checkboxTag) { + if(tiddler && this.checkboxTag) { if(this.checkboxInvertTag === "yes") { return !tiddler.hasTag(this.checkboxTag); } else { return tiddler.hasTag(this.checkboxTag); } } - if(this.checkboxField || this.checkboxIndex) { + if(tiddler && (this.checkboxField || this.checkboxIndex)) { // Same logic applies to fields and indexes var value; if(this.checkboxField) { @@ -206,11 +206,18 @@ CheckboxWidget.prototype.handleChangeEvent = function(event) { } // Set the list field (or index) if specified if(this.checkboxListField || this.checkboxListIndex) { - var listContents, oldPos, newPos; + var fieldContents, listContents, oldPos, newPos; if(this.checkboxListField) { - listContents = tiddler.getFieldList(this.checkboxListField); + fieldContents = tiddler ? tiddler.fields[this.checkboxListField] : undefined; } else { - listContents = $tw.utils.parseStringArray(this.wiki.extractTiddlerDataItem(this.checkboxTitle,this.checkboxListIndex) || "") || []; + fieldContents = this.wiki.extractTiddlerDataItem(this.checkboxTitle,this.checkboxListIndex); + } + if($tw.utils.isArray(fieldContents)) { + // Make a copy so we can modify it without changing original that's refrenced elsewhere + listContents = fieldContents.slice(0); + } else { + listContents = $tw.utils.parseStringArray(fieldContents) || []; + // No need to copy since parseStringArray returns a fresh array, not refrenced elsewhere } oldPos = notValue ? listContents.indexOf(notValue) : -1; newPos = value ? listContents.indexOf(value) : -1; diff --git a/editions/test/tiddlers/tests/test-checkbox-widget.js b/editions/test/tiddlers/tests/test-checkbox-widget.js index 30df75bbd..a6572afd6 100644 --- a/editions/test/tiddlers/tests/test-checkbox-widget.js +++ b/editions/test/tiddlers/tests/test-checkbox-widget.js @@ -234,6 +234,38 @@ Tests the checkbox widget thoroughly. }, ]; + // https://github.com/Jermolene/TiddlyWiki5/issues/6871 + const listModeTestsWithListField = ( + listModeTests + .filter(data => data.widgetText.includes("listField='colors'")) + .map(data => { + const newData = { + ...data, + tiddlers: data.tiddlers.map(tiddler => ({...tiddler, list: tiddler.colors, colors: undefined})), + widgetText: data.widgetText.replace("listField='colors'", "listField='list'"), + expectedChange: { + "Colors": { list: data.expectedChange.Colors.colors.split(' ') } + }, + } + return newData; + }) + ); + const listModeTestsWithTagsField = ( + listModeTests + .filter(data => data.widgetText.includes("listField='colors'")) + .map(data => { + const newData = { + ...data, + tiddlers: data.tiddlers.map(tiddler => ({...tiddler, tags: tiddler.colors, colors: undefined})), + widgetText: data.widgetText.replace("listField='colors'", "listField='tags'"), + expectedChange: { + "Colors": { tags: data.expectedChange.Colors.colors.split(' ') } + }, + } + return newData; + }) + ); + const indexListModeTests = listModeTests.map(data => { const newData = {...data}; const newName = data.testName.replace('list mode', 'index list mode'); @@ -453,6 +485,8 @@ Tests the checkbox widget thoroughly. const checkboxTestData = fieldModeTests.concat( indexModeTests, listModeTests, + listModeTestsWithListField, + listModeTestsWithTagsField, indexListModeTests, filterModeTests, ); @@ -495,7 +529,7 @@ Tests the checkbox widget thoroughly. for (const fieldName of Object.keys(change)) { const expectedValue = change[fieldName]; const fieldValue = tiddler.fields[fieldName]; - expect(fieldValue).toBe(expectedValue); + expect(fieldValue).toEqual(expectedValue); } } }) From c5d3d4c26e8fe27f272dda004aec27d6b66c4f60 Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Fri, 23 Sep 2022 18:07:46 +0100 Subject: [PATCH 2/7] Disable wiki indexers in safe mode --- boot/boot.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boot/boot.js b/boot/boot.js index f902e9956..ba519a2cf 100644 --- a/boot/boot.js +++ b/boot/boot.js @@ -2416,7 +2416,7 @@ $tw.boot.initStartup = function(options) { $tw.utils.registerFileType("application/epub+zip","base64",".epub"); $tw.utils.registerFileType("application/octet-stream","base64",".octet-stream"); // Create the wiki store for the app - $tw.wiki = new $tw.Wiki(); + $tw.wiki = new $tw.Wiki($tw.safeMode && {enableIndexers: []}); // Install built in tiddler fields modules $tw.Tiddler.fieldModules = $tw.modules.getModulesByTypeAsHashmap("tiddlerfield"); // Install the tiddler deserializer modules From 166a1565843878083fb1eba47c73b8e67b78400d Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Fri, 23 Sep 2022 18:08:28 +0100 Subject: [PATCH 3/7] Fix typo: Safe mode should prevent globally disabling parser rules --- core/modules/parsers/wikiparser/wikiparser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/modules/parsers/wikiparser/wikiparser.js b/core/modules/parsers/wikiparser/wikiparser.js index 90a3e7446..8d6c442de 100644 --- a/core/modules/parsers/wikiparser/wikiparser.js +++ b/core/modules/parsers/wikiparser/wikiparser.js @@ -116,7 +116,7 @@ WikiParser.prototype.loadRemoteTiddler = function(url) { */ WikiParser.prototype.setupRules = function(proto,configPrefix) { var self = this; - if(!$tw.safemode) { + if(!$tw.safeMode) { $tw.utils.each(proto,function(object,name) { if(self.wiki.getTiddlerText(configPrefix + name,"enable") !== "enable") { delete proto[name]; From 53d229592df76c6dd607e40be5bea4d5e063c48e Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Fri, 23 Sep 2022 18:09:16 +0100 Subject: [PATCH 4/7] Optimise wiki.getTiddler() --- boot/boot.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/boot/boot.js b/boot/boot.js index ba519a2cf..6bfa6495a 100644 --- a/boot/boot.js +++ b/boot/boot.js @@ -1230,13 +1230,16 @@ $tw.Wiki = function(options) { this.getTiddler = function(title) { if(title) { var t = tiddlers[title]; - if(t instanceof $tw.Tiddler) { + if(t !== undefined) { return t; - } else if(title !== undefined && shadowTiddlers[title]) { - return shadowTiddlers[title].tiddler; + } else { + var s = shadowTiddlers[title]; + if(s !== undefined) { + return s.tiddler; + } } - return undefined; } + return undefined; }; // Get an array of all tiddler titles From 0a00da6db9b0a818ec5ab90fd6cfc5544c4e6533 Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Fri, 23 Sep 2022 18:09:45 +0100 Subject: [PATCH 5/7] Optimise fake dom Object.setPrototypeOf() appears to be significantly faster --- core/modules/utils/fakedom.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/modules/utils/fakedom.js b/core/modules/utils/fakedom.js index 590a7ffe0..d28161ac6 100755 --- a/core/modules/utils/fakedom.js +++ b/core/modules/utils/fakedom.js @@ -42,7 +42,7 @@ var TW_TextNode = function(text) { this.textContent = text + ""; }; -TW_TextNode.prototype = Object.create(TW_Node.prototype); +Object.setPrototypeOf(TW_TextNode,TW_Node.prototype); Object.defineProperty(TW_TextNode.prototype, "nodeType", { get: function() { @@ -67,7 +67,7 @@ var TW_Element = function(tag,namespace) { this.namespaceURI = namespace || "http://www.w3.org/1999/xhtml"; }; -TW_Element.prototype = Object.create(TW_Node.prototype); +Object.setPrototypeOf(TW_Element,TW_Node.prototype); Object.defineProperty(TW_Element.prototype, "style", { get: function() { From 81ac9874846b3ead275f67010fcfdb49f3d2f43c Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Sat, 24 Sep 2022 08:28:16 +0100 Subject: [PATCH 6/7] Optimise variable prototype chain handling With this improvement and 53d229592df76c6dd607e40be5bea4d5e063c48e I'm measuring a 10-15% performance improvement between v5.2.3 and master using https://github.com/Jermolene/tiddlywiki-performance-test-rig --- core/modules/widgets/importvariables.js | 5 ++++- core/modules/widgets/widget.js | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/core/modules/widgets/importvariables.js b/core/modules/widgets/importvariables.js index 0cda68138..a73abfdcf 100644 --- a/core/modules/widgets/importvariables.js +++ b/core/modules/widgets/importvariables.js @@ -39,7 +39,10 @@ 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(); + this.variables = Object.create(null); + if(this.parentWidget) { + Object.setPrototypeOf(this.variables,this.parentWidget.variables); + } // Get our parameters this.filter = this.getAttribute("filter"); // Compute the filter diff --git a/core/modules/widgets/widget.js b/core/modules/widgets/widget.js index 0aefbada8..7034c9f37 100755 --- a/core/modules/widgets/widget.js +++ b/core/modules/widgets/widget.js @@ -38,9 +38,10 @@ Widget.prototype.initialise = function(parseTreeNode,options) { this.parseTreeNode = parseTreeNode; this.wiki = options.wiki; this.parentWidget = options.parentWidget; - this.variablesConstructor = function() {}; - this.variablesConstructor.prototype = this.parentWidget ? this.parentWidget.variables : {}; - this.variables = new this.variablesConstructor(); + this.variables = Object.create(null); + if(this.parentWidget) { + Object.setPrototypeOf(this.variables,this.parentWidget.variables); + } this.document = options.document; this.attributes = {}; this.children = []; From 8ebb9ef4427b04518cdfb9cc9cc56c900ee85f98 Mon Sep 17 00:00:00 2001 From: "jeremy@jermolene.com" Date: Sat, 24 Sep 2022 10:55:22 +0100 Subject: [PATCH 7/7] Typo from a981f8ccfe80634eb15b9ed33393775a2d77994f Fixes #6955 --- editions/tw5.com/tiddlers/system/wikitext-macros.tid | 1 + 1 file changed, 1 insertion(+) diff --git a/editions/tw5.com/tiddlers/system/wikitext-macros.tid b/editions/tw5.com/tiddlers/system/wikitext-macros.tid index c63fe5f8c..70f62bfb7 100644 --- a/editions/tw5.com/tiddlers/system/wikitext-macros.tid +++ b/editions/tw5.com/tiddlers/system/wikitext-macros.tid @@ -70,6 +70,7 @@ That renders as: That renders as: <$macrocall $name="__src__"/> + \end