If you add a new field value for platform, OS, and other fields with a comma like "HACK,HACK" the search for bugs with that value will be broken because the search engine split values by commas. A simple check for commas in new values should fix this.

Multiple custom field values were being joined and later split by commas to construct an "anyexact" search. This can be fixed by keeping them in an array instead of joining them. The array has to be dereferenced when printing debug text.

Comment on attachment 277760 [details] [diff] [review] v1 >@@ -1111,7 +1110,7 @@ > }, > ",anyexact" => sub { > my @list; >- foreach my $w (split(/,/, $v)) { >+ foreach my $w (@$v) { I'm pretty sure this will break using anyexact from the Boolean Charts. >+ my $f = $params->param("field$chart-$row-$col"); >+ my $t = $params->param("type$chart-$row-$col"); >+ my $v = $params->param("value$chart-$row-$col"); >+ $v = join ',', @$v if (ref $v eq 'ARRAY'); You can't redefine $f, $t, and $v. At the least, you can't "my" them again. >@@ -1366,6 +1367,7 @@ >+ my $v = join ',', @$v if (ref $v eq 'ARRAY'); > push(@debugdata, "$f / $t / $v / " . Same here.

This version uses more descriptive variable names in debug blocks and supports both arrays and strings for anyexact searches. Anyexact searches made from the list selection work correctly. Anyexact searches from the Boolean Charts have no way to escape commas, but can be replaced with multiple exact searches. Custom field values with commas can be successfully searched for in the Boolean Charts by exact, but not by anyexact.

Comment on attachment 277804 [details] [diff] [review] v2 $rhs is treated as a string above your code. Also, you should do: join(',', @$rhs) if ref $rhs eq 'ARRAY' That avoids ambiguity of what the "if" is "iffing". :-) Same for $debug_v at the bottom. Any idea why sometimes we use "|" in the debug, and sometimes we use "/"?

Dereferenced $rhs at the correct place, and clarified if statements.

Comment on attachment 278124 [details] [diff] [review] v3 >+ @values = split(/,/, $v); Nit: Just do ',' instead of /,/. >@@ -1351,6 +1358,7 @@ > $q = $dbh->quote($v); > trick_taint($q); > my $rhs = $v; >+ $rhs = join(',', @$rhs) if ref $rhs eq 'ARRAY'; > $rhs =~ tr/,//; Wait, so you join them with commas and then remove all the commas? That doesn't make sense.

(In reply to comment #6) > >+ @values = split(/,/, $v); > > Nit: Just do ',' instead of /,/. I personally prefer the /,/ syntax and is the syntax used in |perldoc -f split|. Keep it!

> $rhs =~ tr/,//; This line wasn't actually deleting commas since it lacked the 'd' flag. So in the unpatched version, searching for the name of an operation would invoke that operation if it was listed earlier than the correct operation in @funcdefs. For example, a Boolean Chart search for "OS/Version" "contains any of the words" "anyexact,Windows" would invoke an anyexact search instead of an anywords search and return nothing. I have added stricter conditions to the keys in @funcdefs to ensure that search type will not be matched by value text. This also makes it unnecessary to remove commas from $rhs. Requiring patterns to match from the beginning of the string is probably safer than just matching the function name.

Hey Jesse. Now I think you're overlapping with bug 67036. Actually, I think this bug might even be a duplicate of that bug. What do you think?

Yes, this is the same bug. Joel's approach is probably the most robust. Perhaps it is time to revive it.