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.
This commit is contained in:
Robin Munn 2022-09-23 00:52:55 +07:00 committed by GitHub
parent 50f54ba6ca
commit 51bdf60ee8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 47 additions and 6 deletions

View File

@ -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;

View File

@ -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);
}
}
})