Last modified: 2011-07-26 21:36:24 UTC
Created attachment 8370 [details] proposed patch I've done 4 fixes for the sorttable script: # the digitClass join versions were inverted: for a maxLength > 1 it should be (|||), not [] # the regexp escape for digits in the ts_number_transform_table was a bit, er, strange. Although it worked, I think my version is more common. # the non breaking space \xa0 is included in the whitespace shortcut \s (or is there a browser bug?), so I've changed [\xa0\s] to \s # most important: As recommended in http://en.wikipedia.org/wiki/Percent_sign, there may be some whitespaces between the number and the "%". See also Bug 15422 for that, I'm not sure wheter it's included there.
Can you make a .diff of the patch, so it's easier for others to review and apply? More info: http://www.mediawiki.org/wiki/Subversion#Making_a_diff
Created attachment 8371 [details] proposed patch MIME-type misdetection. Bugzilla-bug?
Comment on attachment 8371 [details] proposed patch >Index: wikibits.js >=================================================================== >--- wikibits.js (Revision 83609) >+++ wikibits.js (Arbeitskopie) >@@ -626,7 +626,7 @@ > for ( var i = rowStart; i < table.rows.length; i++ ) { > if ( table.rows[i].cells.length > column ) { > itm = ts_getInnerText(table.rows[i].cells[column]); >- itm = itm.replace(/^[\s\xa0]+/, '').replace(/[\s\xa0]+$/, ''); >+ itm = itm.replace(/^\s+/, '').replace(/\s+$/, ''); > if ( itm != '' ) { > break; > } >@@ -661,7 +661,7 @@ > keyText = ''; > } > var oldIndex = ( reverse ? -j : j ); >- var preprocessed = preprocessor( keyText.replace(/^[\s\xa0]+/, '').replace(/[\s\xa0]+$/, '') ); >+ var preprocessed = preprocessor( keyText.replace(/^\s+/, '').replace(/\s+$/, '') ); > > newRows[newRows.length] = new Array( row, preprocessed, oldIndex ); > } else { >@@ -740,17 +740,16 @@ > for ( var digit in ts_number_transform_table ) { > // Escape regex metacharacters > digits.push( >- digit.replace( /[\\\\$\*\+\?\.\(\)\|\{\}\[\]\-]/, >- function( s ) { return '\\' + s; } ) >+ digit.replace( /([{}()|.?*+^$\[\]\\-])/g, "\\$1" ) > ); > if ( digit.length > maxDigitLength ) { > maxDigitLength = digit.length; > } > } > if ( maxDigitLength > 1 ) { >- var digitClass = '[' + digits.join( '', digits ) + ']'; >+ var digitClass = '(' + digits.join( '|' ) + ')'; > } else { >- var digitClass = '(' + digits.join( '|', digits ) + ')'; >+ var digitClass = '[' + digits.join( '' ) + ']'; > } > } > >@@ -760,7 +759,7 @@ > "^(" + > "[-+\u2212]?[0-9][0-9,]*(\\.[0-9,]*)?(E[-+\u2212]?[0-9][0-9,]*)?" + // Fortran-style scientific > "|" + >- "[-+\u2212]?" + digitClass + "+%?" + // Generic localised >+ "[-+\u2212]?" + digitClass + "+\s*%?" + // Generic localised > ")$", "i" > ); > };
r85497
Created attachment 8462 [details] proposed patch Could you please add the [\xa0\s]-regex to tablesorter.js? r86088 made my patch obsolete.
Unneccessary. we do $.trim before we use the regex and $.trim removes bsnp (see https://github.com/jquery/jquery/commit/17955cacf4e8c618ca5c2b09b0d2f43df353f683/ )
Sorry, I meant the whitespace(s) between the digits and the percentage sign, see the attached patch. The trim method doesn't handle these.
Ah, right. r87005
Bergi, DieBuche, This patch has been marked FIXME in order to get some tests written for it. (See r85497) Could you write the tests for it? Bergi, If you write the tests I'll commit them and we'll see about getting you commit privs.
per CR on r85497 the code was rewrite. Closing this bug as fixed.
