diff --git a/core/modules/widgets/select.js b/core/modules/widgets/select.js index 07c959550f..ebf84a7ea7 100644 --- a/core/modules/widgets/select.js +++ b/core/modules/widgets/select.js @@ -125,13 +125,12 @@ SelectWidget.prototype.setSelectValue = function() { values = Array.isArray(value) ? value : $tw.utils.parseStringArray(value); for(var i=0; i < select.children.length; i++){ child=select.children[i]; - if(child.children.length === 0){ - child.selected = values.indexOf(child.value) !== -1; - } else { - // grouped options + if(child.tagName && child.tagName.toUpperCase() === "OPTGROUP"){ for(var y=0; y < child.children.length; y++){ child.children[y].selected = values.indexOf(child.children[y].value) !== -1; } + } else { + child.selected = values.indexOf(child.value) !== -1; } } } else { diff --git a/editions/test/tiddlers/tests/test-select-widget.js b/editions/test/tiddlers/tests/test-select-widget.js new file mode 100644 index 0000000000..c99f012675 --- /dev/null +++ b/editions/test/tiddlers/tests/test-select-widget.js @@ -0,0 +1,144 @@ +/*\ +title: test-select-widget.js +type: application/javascript +tags: [[$:/tags/test-spec]] + +Tests the select widget, focused on multi-select refresh behaviour. + +\*/ + +"use strict"; + +describe("Select widget", function() { + + var widget = require("$:/core/modules/widgets/widget.js"); + + // Helpers reused from: test-widget.js and test-checkbox-widget.js + function createWidgetNode(parseTreeNode,wiki) { + return new widget.widget(parseTreeNode,{ + wiki: wiki, + document: $tw.fakeDocument + }); + } + + function parseText(text,wiki,options) { + var parser = wiki.parseText("text/vnd.tiddlywiki",text,options); + return parser ? {type: "widget", children: parser.tree} : undefined; + } + + function renderWidgetNode(widgetNode) { + $tw.fakeDocument.setSequenceNumber(0); + var wrapper = $tw.fakeDocument.createElement("div"); + widgetNode.render(wrapper,null); + return wrapper; + } + + function refreshWidgetNode(widgetNode,wrapper,changes) { + var changedTiddlers = {}; + if(changes) { + $tw.utils.each(changes,function(title) { + changedTiddlers[title] = true; + }); + } + widgetNode.refresh(changedTiddlers,wrapper,null); + } + + // Don't pass fakedom elements to Jasmine matchers. The matchers pretty-print + // values on assertion which walks the element.style Proxy with Symbol keys and + // crashes inside convertStyleNameToPropertyName. + // TODO: once issue: "fakedom style Proxy guards against non-string property keys" lands + // revert these workarounds to idiomatic matchers (toBe(null), toBeUndefined()). + function findSelectDom(node) { + if(node.tag === "select") return node; + if(node.children) { + for(var i = 0; i < node.children.length; i++) { + var found = findSelectDom(node.children[i]); + if(found) return found; + } + } + return null; + } + + function selectedFlags(parent) { + var result = []; + for(var i = 0; i < parent.children.length; i++) { + result.push(!!parent.children[i].selected); + } + return result; + } + + // Regression test for https://github.com/TiddlyWiki/TiddlyWiki5/issues/9839 + // PR #8093 added support but used `child.children.length === 0` to + // distinguish a plain . That heuristic misfires for + // any " + + "" + + "" + + ""; + var widgetNode = createWidgetNode(parseText(widgetText,wiki),wiki); + var wrapper = renderWidgetNode(widgetNode); + var select = findSelectDom(wrapper); + expect(select === null).toBe(false); + expect(select.children.length).toBe(3); + + // After initial render, options matching the field value should be selected. + // foo (idx 0), bar (idx 1), $:/mumble (idx 2). + expect(selectedFlags(select)).toEqual([true,false,true]); + + // Change the stored field value and refresh - this is where the bug surfaced: + // the "$:/mumble" option (with an child) was wrongly skipped. + wiki.addTiddler({title: "Picks", mylist: "bar $:/mumble"}); + refreshWidgetNode(widgetNode,wrapper,["Picks"]); + + expect(selectedFlags(select)).toEqual([false,true,true]); + // The inner must not be touched - .selected is meaningful only on across refresh, including $:/-prefixed entries with inline HTML",function() { + var wiki = $tw.test.wiki(); + wiki.addTiddlers([ + {title: "Picks", mylist: "1 $:/mumble"} + ]); + // The "high" group mixes a plain option with a $:/-prefixed option whose + // content is wrapped in an auto-link - same pattern that broke #9839 + // for top-level options, exercised here inside an . + var widgetText = "<$select tiddler='Picks' field='mylist' multiple>" + + "" + + "" + + "" + + "" + + "" + + "" + + "$:/mumble" + + "" + + ""; + var widgetNode = createWidgetNode(parseText(widgetText,wiki),wiki); + var wrapper = renderWidgetNode(widgetNode); + var select = findSelectDom(wrapper); + expect(select === null).toBe(false); + expect(select.children.length).toBe(2); + + // Initial state: "1" and "$:/mumble" selected + expect(selectedFlags(select.children[0])).toEqual([true,false]); + expect(selectedFlags(select.children[1])).toEqual([false,true]); + + wiki.addTiddler({title: "Picks", mylist: "2 4"}); + refreshWidgetNode(widgetNode,wrapper,["Picks"]); + expect(selectedFlags(select.children[0])).toEqual([false,true]); + expect(selectedFlags(select.children[1])).toEqual([true,false]); + }); + +});