FilteringSelect does not support options with values that contain commas

[hborders]

I have a dijit.form.FilteringSelect? that contains options with commas (',') in their value. The FilteringSelect? fails to update its label or value when its attr method is called with a value containing a comma. Here is an example:

<select dojoType="dijit.form.FilteringSelect" id="withCommas">
  <option value="1,2" selected="selected">one or two</option>
  <option value="3">three</option>
</select>

When the page initially loads, the FilteringSelect? should have 'one or two' in its label, but it shows nothing. Further if I make the following call from firebug nothing happens either:

dijit.byId('withCommas').attr('value', '1,2');

I believe the problem lies with dojo.query's parsing of attribute-value selectors. It seems to separate queries that contain commas into separate individual queries regardless of where the comma occurs. The query parser should ignore commas within attribute-value literals.

[Douglas Hays]

This appears to be a dojo.query bug. Consider the following snipit:

<select>
<option value=",">,</option>
</select>
<input type=button onclick=alert(dojo.query("option[value=',']")[0]) value="query option with ,">

dojo.query fails to find the option node on IE6, IE7, FF2, FF3, and Safari 3, but does find it on Safari 4.
I tested on the latest trunk = [17488].

[marcpeterson]

I was able to get dojo.query to search for results with commas, but for some reason it doesn't like the _ComboBoxDataStore data store.

In dijit.form._ComboBoxDataStore.fetchItemByIdentity(), after the line

var item = dojo.query("option[value='" + args.identity + "']", this.root)[0];

You can add the following as a backup query (the original is still needed as it picks up integers, this doesn't)

if(!item){
	for(var i=0; i<this.root.length; i++){
		if(this.root[i].innerHTML == args.identity){
			item = this.root[i];
			break;
		}
	}
}

This looks at each of stored dom nodes and compares their contents to the search string. I'm not sure why it doesn't work on integers.

[wdoekes]

I had to add the following to get it to work in both IE6 and FF3:

--- form/ComboBox.js	(revision 20788)
+++ form/ComboBox.js	(working copy)
@@ -1124,7 +1124,19 @@
 		//	|		{identity: "CA", onItem: function(item){...}
 		//
 		//		Call `onItem()` with the DOM node `<option value="CA">California</option>`
-		var item = dojo.query("option[value='" + args.identity + "']", this.root)[0];
+		var item = false;
+		try {
+			item = dojo.query("option[value='" + args.identity + "']", this.root)[0];
+		} catch(e) {}
+		if(!item || typeof(item.selectedIndex) !== undefined){ // FF3 can return a HTMLSelectWidget..
+			for(var i=0; i<this.root.length; i++){
+				if(this.root[i].innerHTML == args.identity){
+					item = this.root[i];
+					break;
+				}
+			}
+		}
+
 		args.onItem(item);
 	},
 	

IE6 threw an error on the query. Firefox3 returned a HTMLSelectElement widget instead of the HTMLOption.

Regards, Walter Doekes OSSO B.V.

[Chris Mitchell]

please review/triage

[pechanec]

I found this defect in 1.6.1 in Firefox 3 and 4 too. Would be great for me to be fixed.

[bill]

It may be working better now, or at least on some browsers, since Kris has replaced the acme query engine w/a lightweight engine. Although would still fail if the app explicitly loaded the acme engine.

[ben hockey]

#7479 seems related and has a patch that claims to have dojo.query not parse the values for attributes.

Should this ticket be closed either as duplicate or wontfix?

[bill]

Actually, the reported problem is fixed in 1.7, because FilteringSelect was refactored to not depend on dojo.query() so much, in my conversion for it to support dojo.store. As for the dojo.query() bug, let's assume that is reported in other tickets like the one you mentioned.

[wdoekes]

Possible fix, in case you're still using 1.2:

--- static/dojo/1.2.x/dojo/_base/query.js	2017-09-15 13:13:13.860344455 +0200
+++ static/dojo/1.2.x/dojo/_base/query.js.patched	2017-09-15 13:13:42.980344981 +0200
@@ -993,7 +993,18 @@ dojo.require("dojo._base.NodeList");
 			// and return a dispatcher that will merge the parts when run
 
 			// var parts = query.split(", ");
-			var parts = query.split(/\s*,\s*/);
+			// var parts = query.split(/\s*,\s*/);
+			var parts = [];
+			var inquery = ',' + query;
+			var match = null;
+			do {
+				match = inquery.match(/,\s*((?:[^"',]+|"[^"]*"|'[^']*')+)\s*/);
+				if (match) {
+					parts.push(match[1]);
+					inquery = inquery.substring(match[0].length);
+				}
+			} while (match);
+
 			var tf = function(root){
 				var pindex = 0; // avoid array alloc for every invocation
 				var ret = [];

This should split form, options[value=','] into form and options[value=','] instead of splitting it into three components.

Regards, Walter Doekes OSSO B.V.