TableSorter: Fix column order when collecting headers

Before we did a 'lazy' explode of the columns and rows, a side
effect of this was that a header (A) on a row >= 1 would get a
headerIndex that was higher than that of the header (B) with a
rowspan, that would visually be after header (A).

This caused headers to control the wrong column

Bug: 53211
Change-Id: I852d2860951a4e48f7fb2f6bf8c26b986af3e727

diff --git a/resources/jquery/jquery.tablesorter.js b/resources/jquery/jquery.tablesorter.js
index b3d7bb3..f9ee268 100644 (file)
--- a/resources/jquery/jquery.tablesorter.js
+++ b/resources/jquery/jquery.tablesorter.js
@@ -289,7 +289,6 @@
        function buildHeaders( table, msg ) {
                var maxSeen = 0,
                        colspanOffset = 0,
-                       longest,
                        columns,
                        i,
                        $tableHeaders = $( [] ),
@@ -297,27 +296,47 @@
                if ( $tableRows.length <= 1 ) {
                        $tableHeaders = $tableRows.children( 'th' );
                } else {
-                       // We need to find the cells of the row containing the most columns
                        var rowspan,
-                               headersIndex = [];
-                       $tableRows.each( function ( rowIndex ) {
-                               $.each( this.cells, function( index2, cell ) {
+                               colspan,
+                               headerCount,
+                               longestTR,
+                               matrixRowIndex,
+                               matrixColumnIndex,
+                               exploded = [];
+
+                       // Loop through all the dom cells of the thead
+                       $tableRows.each( function ( rowIndex, row ) {
+                               $.each( row.cells, function( columnIndex, cell ) {
                                        rowspan = Number( cell.rowSpan );
-                                       for ( i = 0; i < rowspan; i++ ) {
-                                               if ( headersIndex[rowIndex+i] === undefined ) {
-                                                       headersIndex[rowIndex+i] = $( [] );
+                                       colspan = Number( cell.colSpan );
+
+                                       // Skip the spots in the exploded matrix that are already filled
+                                       while ( exploded[rowIndex] && exploded[rowIndex][columnIndex] !== undefined ) {
+                                               ++columnIndex;
+                                       }
+
+                                       // Find the actual dimensions of the thead, by placing each cell
+                                       // in the exploded matrix rowspan times colspan times, with the proper offsets
+                                       for ( matrixColumnIndex = columnIndex; matrixColumnIndex < columnIndex + colspan; ++matrixColumnIndex ) {
+                                               for ( matrixRowIndex = rowIndex; matrixRowIndex < rowIndex + rowspan; ++matrixRowIndex ) {
+                                                       if ( !exploded[matrixRowIndex] ) {
+                                                               exploded[matrixRowIndex] = [];
+                                                       }
+                                                       exploded[matrixRowIndex][matrixColumnIndex] = cell;
                                                }
-                                               headersIndex[rowIndex+i].push( cell );
                                        }
                                } );
                        } );
-                       $.each( headersIndex, function ( index, cellArray ) {
-                               if ( cellArray.length >= maxSeen ) {
-                                       maxSeen = cellArray.length;
-                                       longest = index;
+                       // We want to find the row that has the most columns (ignoring colspan)
+                       $.each( exploded, function ( index, cellArray ) {
+                               headerCount = $.unique( $(cellArray) ).length;
+                               if ( headerCount >= maxSeen ) {
+                                       maxSeen = headerCount;
+                                       longestTR = index;
                                }
                        } );
-                       $tableHeaders = headersIndex[longest];
+                       // We cannot use $.unique() here because it sorts into dom order, which is undesirable
+                       $tableHeaders = $( uniqueElements( exploded[longestTR] ) );
                }
 
                // as each header can span over multiple columns (using colspan=N),
@@ -405,6 +424,17 @@
                return false;
        }
 
+
+       function uniqueElements( array ) {
+               var uniques = [];
+               $.each( array, function( index, elem ) {
+                       if ( elem !== undefined && $.inArray( elem, uniques ) === -1 ) {
+                               uniques.push( elem );
+                       }
+               } );
+               return uniques;
+       }
+
        function setHeadersCss( table, $headers, list, css, msg, columnToHeader ) {
                // Remove all header information and reset titles to default message
                $headers.removeClass( css[0] ).removeClass( css[1] ).attr( 'title', msg[1] );

diff --git a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
index f73fd7b..d8a7d6a 100644 (file)
--- a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
+++ b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js
@@ -1150,6 +1150,28 @@
                );
        } );
 
+       QUnit.test( 'holes in the table headers should not throw JS errors', 2, function ( assert ) {
+               var $table = $(
+                       '<table class="sortable">' +
+                               '<thead>' +
+                               '<tr><th id="A1">A1</th><th>B1</th><th id="C1" rowspan="2">C1</th></tr>' +
+                               '<tr><th id="A2">A2</th></tr>' +
+                               '</thead>' +
+                               '<tr><td>A</td><td>Aa</td><td>Aaa</td></tr>' +
+                               '<tr><td>B</td><td>Ba</td><td>Bbb</td></tr>' +
+                               '</table>'
+               );
+               $table.tablesorter();
+               assert.equal( 0,
+                       $table.find( '#A2' ).prop( 'headerIndex' ),
+                       'A2 should be a sort header'
+               );
+               assert.equal( 1, // should be 2
+                       $table.find( '#C1' ).prop( 'headerIndex' ),
+                       'C1 should be a sort header, but will sort the wrong column'
+               );
+       } );
+
        // bug 41889 - exploding rowspans in more complex cases
        tableTestHTML(
                'Rowspan exploding with row headers',
@@ -1165,6 +1187,22 @@
                ]
        );
 
+       // bug 53211 - exploding rowspans in more complex cases
+       QUnit.test(
+               'Rowspan exploding with row headers and colspans', 1, function ( assert ) {
+               var $table = $( '<table class="sortable">' +
+                       '<thead><tr><th rowspan="2">n</th><th colspan="2">foo</th><th rowspan="2">baz</th></tr>' +
+                       '<tr><th>foo</th><th>bar</th></tr></thead>' +
+                       '<tbody>' +
+                       '<tr><td>1</td><td>foo</td><td>bar</td><td>baz</td></tr>' +
+                       '<tr><td>2</td><td>foo</td><td>bar</td><td>baz</td></tr>' +
+                       '</tbody></table>' );
+
+                       $table.tablesorter();
+                       assert.equal( 2, $table.find( 'tr:eq(1) th:eq(1)').prop('headerIndex'), 'Incorrect index of sort header' );
+               }
+       );
+
        tableTestHTML(
                'Rowspan exploding with colspanned cells',
                '<table class="sortable">' +