jenkins-bot has submitted this change and it was merged.

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, 26 insertions(+), 5 deletions(-)

Approvals:
  Helder.wiki: Looks good to me, but someone else must approve
  Nikerabbit: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/src/jquery/jquery.tablesorter.js 
b/resources/src/jquery/jquery.tablesorter.js
index 5b1e2a7..ae28536 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..92dad9f 100644
--- a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
+++ b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
@@ -1161,12 +1161,33 @@
                );
                $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,
-                       'C1 should be a sort header, but will sort the wrong 
column'
+                       2,
+                       'C1 should be a sort header'
+               );
+       } );
+
+       // 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 not be a sort header'
                );
        } );
 

-- 
To view, visit https://gerrit.wikimedia.org/r/152225
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie551cd62275dd80560643131d68435166732d367
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: TheDJ <hartman.w...@gmail.com>
Gerrit-Reviewer: Helder.wiki <helder.w...@gmail.com>
Gerrit-Reviewer: Jack Phoenix <j...@countervandalism.net>
Gerrit-Reviewer: Nikerabbit <niklas.laxst...@gmail.com>
Gerrit-Reviewer: TheDJ <hartman.w...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to