[FNR] String comparison bug in BaseStringFilterConfig

[crehbein]

Hi,

I just noticed there's a bug in the isFiltered() method in BaseStringFilterConfig. isFiltered() is always returning true in a particular case where I'm setting up a filter to include all models with a value of "C" for some key, for example. The line that checks test vs. value is:



return v.toLowerCase().indexOf(t) == -1;


if "C" is first converted to lower case, then indexOf("C") will always return -1, so it'd be filtered out.

This bug is evident in the Remote Filter Grid example on your site by adding a filter for 'A'. I'd expect to see Apple and all the other companies whose names start with a capital 'A', but I get no results. Searching for 'a', on the other hand, works as expected.

Thanks,
- C

PS - I take it back, searching for 'a' in the remote filter grid example doesn't work either, as it includes "Apple, Inc." in the results which doesn't contain a lower-case 'a'. What's the reason for the case conversion?

[sven]

PS - I take it back, searching for 'a' in the remote filter grid example doesn't work either, as it includes "Apple, Inc." in the results which doesn't contain a lower-case 'a'. What's the reason for the case conversion?
The clientside filtering is doing that too. I will change the serverside filtering to be exactly the same.

[crehbein]

Hi Sven, thanks for looking into this.

What's the change going to be? I sort of assumed the BaseStringFilterConfig is the same for remote/local grids and both would have the same problems.

[sven]

This should be fixed in SVN as of revision 2228 now. Can you validate this?

[crehbein]

I just synched up and looked at the code (haven't tried running against the new version yet). It looks like it should fix the problem. One quick question, though - is the intention to provide case-insensitive matching? I'm still not sure what the motivation is to convert the test and value strings to lower-case.

[sven]

Yes, this is how it was implemented now. We cannot change this anymore now as this would be a braking change. we could have changed this in the prerelease phase.

[crehbein]

That's cool, as long as I know what to tell our users to expect. A nice feature would let us configure case-sensitivity when creating a StringFilter but I don't see it being that critical.

Thanks for the help!
- Chris

[sven]

You could also use an own StringFitler/BaseStringFilterConfig class and override the methods with your own logic, if needed.