TheDJ has uploaded a new change for review. https://gerrit.wikimedia.org/r/152225
Change subject: jquery.tablesorter: Only look at th's for headers ...................................................................... jquery.tablesorter: Only look at th's for headers If there would be td cells inside the thead, then these would be counted too for longest header row. This was not possible in wikitext, but was possible in MediaWiki tables. And $.unique sets fields to undefined instead of deleting them, which causes a nasty tough to spot counting mistake (which was even known due to a testcase having a comment confirming that the count was off) Bug: 53527 Change-Id: Ie551cd62275dd80560643131d68435166732d367 --- M resources/src/jquery/jquery.tablesorter.js M tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js 2 files changed, 25 insertions(+), 4 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/25/152225/1 diff --git a/resources/src/jquery/jquery.tablesorter.js b/resources/src/jquery/jquery.tablesorter.js index 5b1e2a7..5483d6c 100644 --- a/resources/src/jquery/jquery.tablesorter.js +++ b/resources/src/jquery/jquery.tablesorter.js @@ -331,14 +331,14 @@ } ); // We want to find the row that has the most columns (ignoring colspan) $.each( exploded, function ( index, cellArray ) { - headerCount = $.unique( $( cellArray ) ).length; + headerCount = $( uniqueElements( cellArray) ).filter( 'th' ).length; if ( headerCount >= maxSeen ) { maxSeen = headerCount; longestTR = index; } } ); // We cannot use $.unique() here because it sorts into dom order, which is undesirable - $tableHeaders = $( uniqueElements( exploded[longestTR] ) ); + $tableHeaders = $( uniqueElements( exploded[longestTR] ) ).filter( 'th' ); } // as each header can span over multiple columns (using colspan=N), diff --git a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js index 2fd3ce9..98c5a6e 100644 --- a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js @@ -1161,11 +1161,32 @@ ); $table.tablesorter(); assert.equal( $table.find( '#A2' ).prop( 'headerIndex' ), - 0, + undefined, 'A2 should not be a sort header' ); assert.equal( $table.find( '#C1' ).prop( 'headerIndex' ), - 1, + 2, + 'C1 should be a sort header, but will sort the wrong column' + ); + } ); + + // bug 53527 + QUnit.test( 'td cells in thead should not be taken into account for longest row calculation', 2, function ( assert ) { + var $table = $( + '<table class="sortable">' + + '<thead>' + + '<tr><th id="A1">A1</th><th>B1</th><td id="C1">C1</td></tr>' + + '<tr><th id="A2">A2</th><th>B2</th><th id="C2">C2</th></tr>' + + '</thead>' + + '</table>' + ); + $table.tablesorter(); + assert.equal( $table.find( '#C2' ).prop( 'headerIndex' ), + 2, + 'C2 should be a sort header' + ); + assert.equal( $table.find( '#C1' ).prop( 'headerIndex' ), + undefined, 'C1 should be a sort header, but will sort the wrong column' ); } ); -- To view, visit https://gerrit.wikimedia.org/r/152225 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie551cd62275dd80560643131d68435166732d367 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: TheDJ <hartman.w...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits