mediawiki.htmlform: Refactor and clean up

* Use .filter function instead of concatenating strings.
  The end result is the same.

  It's faster and more stable and semantically correct this way
  because using the string syntax is subject to bugs when using
  special characters (you'd have to escape it first, which is
  non-trivial to do in CSS selectors, sometimes impossible).
  So comparing the .name property against a string value directly
  is much easier.

* Use .prop() instead of .attr() where appropiate.
  attr() changes attribute nodes only in the DOM, properties
  reflect the actual live rendered value. This works because of
  a back-compat layer in jQuery, but has been deprecated.

* Use a simple array and create 1 jQuery object, instead of
  creating various temporary objects only to have jQuery merge
  them via .add(). .add() is relatively expensive in that it
  keeps a reference in pushStack and does a lot of logic. Fine
  for convenience, but since we have all the data right here,
  might as well take a more direct approach.

* Remove ill-informed $() calls. This is already a jQuery object
  for the method is a jQuery method. Cloning the object first
  leads to unexpected side-effects and is simply not neccecary.

* Remove use of .live()

Change-Id: Icfe63a4111026661c53639b72e67a4d4fa3d6ac2

diff --git a/resources/src/mediawiki/mediawiki.htmlform.js b/resources/src/mediawiki/mediawiki.htmlform.js
index f7aa7f8..8be1321 100644 (file)
--- a/resources/src/mediawiki/mediawiki.htmlform.js
+++ b/resources/src/mediawiki/mediawiki.htmlform.js
@@ -20,14 +20,18 @@
         * @return {jQuery|null}
         */
        function hideIfGetField( $el, name ) {
-               var sel, $found, $p;
+               var $found, $p,
+                       suffix = name.replace( /^([^\[]+)/, '[$1]' );
+
+               function nameFilter() {
+                       return this.name === name ||
+                               ( this.name === ( 'wp' + name ) ) ||
+                               this.name.slice( -suffix.length ) === suffix;
+               }
 
-               sel = '[name="' + name + '"],' +
-                       '[name="wp' + name + '"],' +
-                       '[name$="' + name.replace( /^([^\[]+)/, '[$1]' ) + '"]';
                for ( $p = $el.parent(); $p.length > 0; $p = $p.parent() ) {
-                       $found = $p.find( sel );
-                       if ( $found.length > 0 ) {
+                       $found = $p.find( '[name]' ).filter( nameFilter );
+                       if ( $found.length ) {
                                return $found;
                        }
                }
@@ -43,7 +47,7 @@
         * @return {Array} 2 elements: jQuery of dependent fields, and test function
         */
        function hideIfParse( $el, spec ) {
-               var op, i, l, v, $field, $fields, func, funcs, getVal;
+               var op, i, l, v, $field, $fields, fields, func, funcs, getVal;
 
                op = spec[0];
                l = spec.length;
@@ -53,15 +57,16 @@
                        case 'NAND':
                        case 'NOR':
                                funcs = [];
-                               $fields = $();
+                               fields = [];
                                for ( i = 1; i < l; i++ ) {
                                        if ( !$.isArray( spec[i] ) ) {
                                                throw new Error( op + ' parameters must be arrays' );
                                        }
                                        v = hideIfParse( $el, spec[i] );
-                                       $fields = $fields.add( v[0] );
+                                       fields.push( v[0] );
                                        funcs.push( v[1] );
                                }
+                               $fields = $( fields );
 
                                l = funcs.length;
                                switch ( op ) {
@@ -143,12 +148,12 @@
                                }
                                v = spec[2];
 
-                               if ( $field.first().attr( 'type' ) === 'radio' ||
-                                       $field.first().attr( 'type' ) === 'checkbox'
+                               if ( $field.first().prop( 'type' ) === 'radio' ||
+                                       $field.first().prop( 'type' ) === 'checkbox'
                                ) {
                                        getVal = function () {
                                                var $selected = $field.filter( ':checked' );
-                                               return $selected.length > 0 ? $selected.val() : '';
+                                               return $selected.length ? $selected.val() : '';
                                        };
                                } else {
                                        getVal = function () {
@@ -185,9 +190,9 @@
         */
        $.fn.goIn = function ( instantToggle ) {
                if ( instantToggle === true ) {
-                       return $( this ).show();
+                       return this.show();
                }
-               return $( this ).stop( true, true ).fadeIn();
+               return this.stop( true, true ).fadeIn();
        };
 
        /**
@@ -199,31 +204,41 @@
         */
        $.fn.goOut = function ( instantToggle ) {
                if ( instantToggle === true ) {
-                       return $( this ).hide();
+                       return this.hide();
                }
-               return $( this ).stop( true, true ).fadeOut();
+               return this.stop( true, true ).fadeOut();
        };
 
        /**
         * Bind a function to the jQuery object via live(), and also immediately trigger
         * the function on the objects with an 'instant' parameter set to true.
+        *
+        * @method liveAndTestAtStart
+        * @deprecated since 1.24 Use .on() and .each() directly.
         * @param {Function} callback
         * @param {boolean|jQuery.Event} callback.immediate True when the event is called immediately,
         *  an event object when triggered from an event.
+        * @return jQuery
+        * @chainable
         */
-       $.fn.liveAndTestAtStart = function ( callback ) {
-               $( this )
+       mw.log.deprecate( $.fn, 'liveAndTestAtStart', function ( callback ) {
+               this
+                       // Can't really migrate to .on() generically, needs knowledge of
+                       // calling code to know the correct selector. Fix callers and
+                       // get rid of this .liveAndTestAtStart() hack.
                        .live( 'change', callback )
                        .each( function () {
                                callback.call( this, true );
                        } );
-       };
+       } );
 
        function enhance( $root ) {
 
-               // Animate the SelectOrOther fields, to only show the text field when
-               // 'other' is selected.
-               $root.find( '.mw-htmlform-select-or-other' ).liveAndTestAtStart( function ( instant ) {
+               /**
+                * @ignore
+                * @param {boolean|jQuery.Event} instant
+                */
+               function handleSelectOrOther( instant ) {
                        var $other = $root.find( '#' + $( this ).attr( 'id' ) + '-other' );
                        $other = $other.add( $other.siblings( 'br' ) );
                        if ( $( this ).val() === 'other' ) {
@@ -231,13 +246,21 @@
                        } else {
                                $other.goOut( instant );
                        }
-               } );
+               }
+
+               // Animate the SelectOrOther fields, to only show the text field when
+               // 'other' is selected.
+               $root
+                       .on( 'change', '.mw-htmlform-select-or-other', handleSelectOrOther )
+                       .each( function () {
+                               handleSelectOrOther.call( this, true );
+                       } );
 
                // Set up hide-if elements
                $root.find( '.mw-htmlform-hide-if' ).each( function () {
-                       var $el = $( this ),
-                               spec = $el.data( 'hideIf' ),
-                               v, $fields, test, func;
+                       var v, $fields, test, func,
+                               $el = $( this ),
+                               spec = $el.data( 'hideIf' );
 
                        if ( !spec ) {
                                return;
@@ -253,7 +276,7 @@
                                        $el.show();
                                }
                        };
-                       $fields.change( func );
+                       $fields.on( 'change', func );
                        func();
                } );