TiddlyWiki5 icon indicating copy to clipboard operation
TiddlyWiki5 copied to clipboard

[BUG] Internal JavaScript Error : TypeError (..) when setting the checkbox widget attribute listField to a "default" field (list, tags,..)

Open Telumire opened this issue 2 years ago • 7 comments

Use this wikitext to reproduce :

* <$checkbox listField="list" checked="A"/>
* <$checkbox listField="list" checked="B"/>
* <$checkbox listField="list" checked="C"/>

Error message when checking more than one of the checkboxes :

"TypeError: can't define array index property past the end of an array with non-writable length"

An error also occur when using a list widget like this :

<$list filter="[is[current]fields[]]" variable="field">
 <$checkbox listField=<<field>> checked="A"/>
</$list>

The error message this time is "TypeError: can't access property "indexOf", list is null"

TiddlyWiki Configuration

  • Tested on v5.2.3 and pre-release 5.2.4

Telumire avatar Aug 03 '22 12:08 Telumire

I can confirm this problem

pmario avatar Aug 03 '22 18:08 pmario

See also https://talk.tiddlywiki.org/t/new-checkbox-listfield-bug-or-user-error/4229

yaisog avatar Aug 09 '22 18:08 yaisog

The cause of this problem is that when I implemented the listField feature, I tried to be efficient by saying "if the field is already stored as a list internally, just modify it in place". But TiddlyWiki's internal tiddler objects are "frozen" against being modified in-place. And my tests didn't catch that because most fields are stored as text internally, but the list field is, for obvious reasons, stored as a list internally.

I'm gotten a fix written for the bug (which involves making a copy of the list, modifying the copy, and then telling TW to please update the list field with this new value), and now I'm working on adding to the tests for the checkbox widget so that it will test the built-in "list" field in addition to any custom fields.

Until the bugfix is released, you can work around this issue by using custom fields, rather than the built-in list field, with the listField property.

rmunn avatar Aug 11 '22 05:08 rmunn

#6897 will fix the problems with using the list field. The second bug report in this issue, where you get a "TypeError: can't access property indexOf, list is null" message, has a different cause. That one is caused by trying to use a date field as if it contained a list. Specifically, the created and modified fields are stored internally as dates, not as text or numbers, and the fields operator gives you a list of all the fields, including . The listField feature will modify the field, by either adding or removing the checked value when you click on the checkbox, and in the case of a date field like modified, you don't want that to happen. Otherwise you'd end up with a field value like modified: "20220811055430402 A" when you check the box, and that would probably cause an error later on when TiddlyWiki tried to parse that text as a date.

To avoid this problem, replace your use of the fields operator with one that excludes fields that you really don't want being modified by the checkbox widget, e.g.

<$list filter="[is[current]fields:exclude[title text created modified]]" variable="field">
 <$checkbox listField=<<field>> checked="A"/>
</$list>

rmunn avatar Aug 11 '22 05:08 rmunn

But TiddlyWiki's internal tiddler objects are "frozen" against being modified in-place.

The tiddler store is immutable, so the Tiddler() functions have to be used to modify it. This allows the refresh mechanism to work reliably. Direct manipulation wouldn't trigger it, which would cause a lot of problems.

pmario avatar Aug 11 '22 06:08 pmario

Which makes perfect sense; the issue was that I called tiddler.getFieldList, knowing that it called parseStringArray and thinking that that meant I was free to modify the resulting array. I didn't realize that parseStringArray, if given an array, returns it directly (which makes perfect sense as well), and that the default list field is stored as an array internally. So my bugfix is to just check if the array I got back from getFieldList is already frozen, and make a copy of it if it was frozen. (If it wasn't frozen, then it's a fresh array just parsed from a string, and not yet used anywhere else, so there's no sense in making a copy, which would be a performance loss for no benefit).

rmunn avatar Aug 11 '22 07:08 rmunn

@rmunn Thanks for working on this issue and for your explanations, I tried to spot the error myself but failed to understand the code, your comments helps a lot. In case you haven't seen it yet, @yaisog has managed to spot another issue : setting the tiddler attribute to a tiddler that doesnt exist causes the error : TypeError: can't access property "getFieldList", tiddler is undefined

E.g :

<$checkbox tiddler="does not exist" checked="true" listField="list" />

Telumire avatar Aug 11 '22 13:08 Telumire

#6897 has been merged now, which will fix most of the problems reported by this bug. There is one remaining problem, which is that if you try to use a checkbox to manage one of TiddlyWiki's default "date" fields (created and/or modified), it will produce a RSoD. I'm still working on that one, and will produce a new PR to address that issue. So please leave this bug open for now until that new PR is created and merged.

rmunn avatar Sep 24 '22 03:09 rmunn

#7448 will fix the issue with date fields like created and modified: if you create a checkbox widget targeting those fields with the listField attribute, they will specifically be ignored and the checkbox widget just won't do anything. Once that PR is merged, this issue can be closed as it will have been completely fixed.

rmunn avatar May 11 '23 15:05 rmunn