jquery.tablesorter: Optimise getElementSortKey() code
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 8 Mar 2019 16:02:44 +0000 (16:02 +0000)
committerTimo Tijhof <krinklemail@gmail.com>
Fri, 8 Mar 2019 21:11:46 +0000 (21:11 +0000)
* Document why this uses $().data() instead of $.data(),
  as the rest of this file does.

* Move single-use $() to where it is needed.

* Use native Image.alt directly instead of going via jQuery
  and attributes. This also obviates the need for a default,
  as the DOM handles that already.

* Remove needless $.makeArray(), use $.map() directly which
  supports the same array-like objects as $.makeArray().

* Use native elem.textContent directly for text nodes,
  instead of $(elem).text().

Change-Id: I92da50e1c01f11585d2bac4f01dba1f671444a63

resources/src/jquery.tablesorter/jquery.tablesorter.js

index a5a1783..b73f430 100644 (file)
         * @return {string}
         */
        function getElementSortKey( node ) {
-               var $node = $( node ),
-                       // Use data-sort-value attribute.
-                       // Use data() instead of attr() so that live value changes
-                       // are processed as well (T40152).
-                       data = $node.data( 'sortValue' );
+               // Get data-sort-value attribute. Uses jQuery to allow live value
+               // changes from other code paths via data(), which reside only in jQuery.
+               // Must use $().data() instead of $.data(), as the latter *only*
+               // accesses the live values, without reading HTML5 attribs first (T40152).
+               var data = $( node ).data( 'sortValue' );
 
                if ( data !== null && data !== undefined ) {
                        // Cast any numbers or other stuff to a string, methods
                        return String( data );
                }
                if ( node.tagName.toLowerCase() === 'img' ) {
-                       return $node.attr( 'alt' ) || ''; // handle undefined alt
+                       return node.alt;
                }
-               return $.makeArray( node.childNodes ).map( function ( elem ) {
+               // Iterate the NodeList (not an array).
+               // Also uses null-return as filter in the same pass.
+               // eslint-disable-next-line no-jquery/no-map-util
+               return $.map( node.childNodes, function ( elem ) {
                        if ( elem.nodeType === Node.ELEMENT_NODE ) {
                                if ( $( elem ).hasClass( 'reference' ) ) {
                                        return null;
-                               } else {
-                                       return getElementSortKey( elem );
                                }
+                               return getElementSortKey( elem );
+                       }
+                       if ( elem.nodeType === Node.TEXT_NODE ) {
+                               return elem.textContent;
                        }
-                       return $.text( elem );
+                       // Ignore other node types, such as HTML comments.
+                       return null;
                } ).join( '' );
        }