Mottie / tablesorter

Github fork of Christian Bach's tablesorter plugin + awesomeness ~
https://mottie.github.io/tablesorter/docs/
2.6k stars 754 forks source link

Wrong behavior with select filter on strings with special character #704

Closed fanturi closed 10 years ago

fanturi commented 10 years ago

Hi,

There seems to have a bug with the select filter. If you have a string in the column with a special character like "-" or "," the select filter doesn't work. If I select an option with "-" or "," in the select it doesn't show any row.

list_with_minus filtered_with_minus

Mottie commented 10 years ago

Hi @fanturi!

I can't duplicate this issue.. I created this demo and it appears to be working properly.

Are you using the latest version? If so, could you please modify the above demo to duplicate the issue.

fanturi commented 10 years ago

Hi,

For example change your examples to add a space character before and after the minus sign. "koala - brown" It's correct in the select list but if you select it the response is incorrect.

TheSin- commented 10 years ago

that makes it a range and that is why it's not working.

Mottie commented 10 years ago

The range filter attempts to match the pattern of - (space-dash-space) to look for a range. If you want to bypass it, I think the easiest method would be to add a textExtraction method to remove the spaces, then add a "filter-parsed" class to the header (demo):

<thead>
    <tr>
        <th class="filter-select filter-parsed">AlphaNumeric</th>
        <th>Numeric</th>
        <th class="filter-select filter-parsed">Animals</th>
        <th>Sites</th>
    </tr>
</thead>

script

$(function () {
    $('table').tablesorter({
        theme: 'blue',
        widgets: ['zebra', 'filter'], 
        textExtraction: function(node, table, cellIndex){
            return $(node).text().replace(/\s/g,'');
        }
    });
});

of course the selects would then not have a space in their options.

fanturi commented 10 years ago

Thanks for the tips but I think it should be the normal behavior. In the select you choose an option and thats all you don't need to define a range. If your needs is to get a range you set a normal filter.

fanturi commented 10 years ago

Hi,

I tested the textExtraction option but it's not working for my case. I have option like this: Executive agency - blabla blabla ( ERTD ) If I use your tips all spaces are removed and the text is all lowercase too. I tried to change the replace by replace(\' - '\g, '-') but it's not working, when I select the option no row is returned. Any ideas ?

Thanks.

Mottie commented 10 years ago

Try using .replace(/\s-\s/g, '');

fanturi commented 10 years ago

It's not working, you can test it in your fiddle: http://jsfiddle.net/Mottie/abkNM/3660/ change animals with this for example: Executive Agency for Small and Medium - sized Enterprises ( EASME )

Is it complex to change the behavior for the select filter to not use the special character and just take the string "as" a string without interpreted it ?

TheSin- commented 10 years ago

changing the default behaviour is not an option.

I have an idea on this as soon as I get to work I'll attempt it on your demo.

Mottie commented 10 years ago

Sorry, the replace should have looked like this .replace(/\s-\s/g, '-') (demo).

TheSin- is looking into working on the code, I won't have time to help until next week.

TheSin- commented 10 years ago

http://jsfiddle.net/aLxsen8t/2/

working, just needed filter-match to force exact matching and ignore other rules because of it.

EDIT spoke to soon, it works for most of them let me track it down.

TheSin- commented 10 years ago

Okay now it's fixed, THIS IS A PROOF OF CONCEPT

http://jsfiddle.net/aLxsen8t/10/

I'm going to work on fixing it in TS next I just wanted to make it work first.

TheSin- commented 10 years ago

line 1348 I added '=' to option value

http://jsfiddle.net/aLxsen8t/12/

this fixes this issue

Still need to make sure it doesn't break other things.

TheSin- commented 10 years ago

okay Again I'm not done testing but this seems more sane. http://jsfiddle.net/aLxsen8t/26/

This makes filter-select respect filter-match

jquery.tablesorter.widget.js - line 1150

fxn = ts.getColumnData( table, wo.filter_functions, columnIndex );

to

fxn = ts.getColumnData( table, wo.filter_functions, columnIndex ) || (c.$headers.filter('[data-column="' + columnIndex + '"]:last').hasClass('filter-match'));

Still need to test more.

fanturi commented 10 years ago

Hi,

It's perfect ! Please tell me when it will be integrated in a new version.

Thanks for your work.

sk2212 commented 10 years ago

Hello,

just try the patch from TheSin. Works for "-" characters but it breaks the filter when inserting other special chars like ')' or '('.

It seems that function call "iExact.search(iFilter)" did not work for unescaped special chars in "iFilter" string.

So I just add something like this:

var myRegexp = /([&\/#,+()$~%.'":*?<>{}])/g; iFilter = iFilter.replace(myRegexp, "\$1");

TheSin- commented 10 years ago

hmm that is very odd, let me look at the code again, I didn't think a regex was being preformed, if so I'll see if I can fix that too. I'm glad the top fix semi worked, as I feel that shouldn't break anything it just makes it so that select-filter and filter-match can co exist.

I'm going to run some tests but I was under the understanding that search was a string search unless a regex was the input. so it's have to be a string made with new Regex or start with / and end with / + modifier. Like /blah/i or something. So why is it treating the string like a regex?

confirmed not working http://jsfiddle.net/aLxsen8t/29/

Adding this around 1160 for testing

// Make iFilter regex safe
var myRegexp = /([&\/\#,+()$~%.'":*?<>{}])/g;
iFilter = iFilter.replace(myRegexp, "\$1");
sk2212 commented 10 years ago

Hmm, just for understanding a updated demo which did not work (with your fix) because your confirmed "not working fiddle" works for me.

http://jsfiddle.net/aLxsen8t/30/

Just try to enter ")" or "(" into "animal" filter column (without filter-select). It breaks tablesorter plugin.

Maybe I miss something?

TheSin- commented 10 years ago

I'm tracking it down just give me some time, I use this to track my attempts and changes so I can commit them once I have a fix I know is 100%. :D

TheSin- commented 10 years ago

updating to latest query.tablesorter.widgets.js before I continue.

http://jsfiddle.net/aLxsen8t/35/

TheSin- commented 10 years ago

Actually I'm going to wait, I just remembered me and @Mottie and by that I mean @Mottie is making changes to how this works, and being able to stack filters and such, it should fix this issue. Once out I'll make sure to test this case again.

Mottie commented 10 years ago

Ok, I've pushed a new update to the working branch to allow adding default filters. Set it up as follows (demo):

$(function () {
    $('table').tablesorter({
        theme: 'blue',
        widgets: ['zebra', 'filter'],
        widgetOptions: {
            filter_defaultFilter: {
                // do exact match on select columns
                '.filter-select' : '={q}'
            }
        }
    });
});
sk2212 commented 10 years ago

Wow...Update from Mottie seems to fix this issue. Special chars are working now within filter query.