From 51bdf60ee868cbf3943d086caa189c5d280b8f54 Mon Sep 17 00:00:00 2001
From: Robin Munn <rmunn@pobox.com>
Date: Fri, 23 Sep 2022 00:52:55 +0700
Subject: [PATCH] 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);
                     }
                 }
             })