r90612 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90611‎ | r90612 | r90613 >
Date:21:54, 22 June 2011
Author:diebuche
Status:ok (Comments)
Tags:
Comment:
Fix tablesorting bug that caused weird interferences between two tables; Make regex more strict to avoid mismatches. All QUnit tests pass now
Modified paths:
  • /trunk/phase3/resources/jquery/jquery.tablesorter.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery/jquery.tablesorter.js
——@@ -378,19 +378,11 @@
379379 mergeSortHelper(array, begin, beginRight, end, sortList);
380380 }
381381
382 - var lastSort = '';
383 -
384382 function multisort( table, sortList, cache ) {
385383 //var sortTime = new Date();
386384
387385 var i = sortList.length;
388 - if ( i == 1 && sortList[0][0] === lastSort) {
389 - // Special case a simple reverse
390 - cache.normalized.reverse();
391 - } else {
392 - mergeSort(cache.normalized, 0, cache.normalized.length, sortList);
393 - }
394 - lastSort = ( sortList.length == 1 ) ? sortList[0][0] : '';
 386+ mergeSort(cache.normalized, 0, cache.normalized.length, sortList);
395387
396388 //benchmark( "Sorting in dir " + order + " time:", sortTime );
397389
——@@ -453,7 +445,7 @@
454446
455447 //Build RegEx
456448 //Any date formated with . , ' - or /
457 - ts.dateRegex[0] = new RegExp(/^\s*\d{1,2}[\,\.\-\/'\s]*\d{1,2}[\,\.\-\/'\s]*\d{2,4}\s*?/i);
 449+ ts.dateRegex[0] = new RegExp(/^\s*\d{1,2}[\,\.\-\/'\s]{1,2}\d{1,2}[\,\.\-\/'\s]{1,2}\d{2,4}\s*?/i);
458450
459451 //Written Month name, dmy
460452 ts.dateRegex[1] = new RegExp('^\\s*\\d{1,2}[\\,\\.\\-\\/\'\\s]*(' + r + ')' + '[\\,\\.\\-\\/\'\\s]*\\d{2,4}\\s*$', 'i');

Follow-up revisions

RevisionCommit summaryAuthorDate
r90618Hopefully fix IE6 regex tablesorter issuediebuche22:21, 22 June 2011
r90630Followup r86088, r87244, r90612: fix jquery.tablesorter for null collation ta...brion23:19, 22 June 2011
r91782MFT to REL_1_18: jquery tablesorter...hashar08:54, 9 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r86088Completely rewritten table sorting script....diebuche21:47, 14 April 2011
r86337r86088: Get rid of eval by implemting a MergeSort algorithm. It's a few ms sl...diebuche19:20, 18 April 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   21:59, 22 June 2011

Confirmed this fixes regressions from r86337 -- the basic sort test cases now run clear. Awesome! :D

#Comment by Krinkle (talk | contribs)   22:16, 22 June 2011

I'm not sure if the problem was introduced in this commit or not, but soe browsers are complaining about regexes in the Test Swarm: 

Exception RegExpError: "Expected ']' in regular expression"

I suck at regexes, but google returns this: http://www.computerperformance.co.uk/Logon/code/code_800A139B.htm

Perhaps someone can check it out ? Looks like most browsers ignore it, but from it could, strictly seen, be a syntax error, or a bug in IE6.

#Comment by Brion VIBBER (talk | contribs)   22:49, 22 June 2011

Yeah, seeing that on IE through 8 at least.

#Comment by Brion VIBBER (talk | contribs)   23:40, 22 June 2011

Several of us poked at this in IRC for a while; I managed to track it down to the collation table building. diebuche says this was a side effect of reading it through mw.config, which left it misdetecting the null (looking for typeof == 'object') and thus accidentally building the regex even when empty... which failed on IE 6/7/8 but not on other browsers.

Fixed in r90630.

Status & tagging log