Various javascript optimizations (fixes bug 39959)

* (bug 39959) Right click to edit section causing browser hangs

  The :has() selector has gotten significantly slower in
  jQuery 1.8.1 however the reason it affected us so much is
  partly because of the very inefficient way we used it.

  Namely duplicated 6 times and evaluated from a live() binding
  In order words, on every instance of the event.

  Optimizing by
  - only select headings, do the :has implicitly by checking
    for the result of .find().
  - document.on(.., selector) instead of .live()
  - remove redundant and overkill 'return false'

  Results in Firefox 15/Chrome 21:
  - up to 130x (one hundred and thirty) faster!

* /resources/* (except for third party libs)
  - Use .on() instead of bind, delegate or live
  - Use .off() instead of unbind, undelegate or die
  - Use document.getElementById instead of $('#' + variable)
    not for performance but for predictability/security.
    (e.g. rel="foo.bar" should select id="foo.bar" not
    id="foo" class="bar")

  - Fix some minor whitespace and code conventions.

Change-Id: I2fefb5376d0de40f4997a3a1763eee23fcd3e7fa

diff --git a/resources/jquery/jquery.makeCollapsible.js b/resources/jquery/jquery.makeCollapsible.js
index 33f8752..0a4d364 100644 (file)
--- a/resources/jquery/jquery.makeCollapsible.js
+++ b/resources/jquery/jquery.makeCollapsible.js
@@ -19,10 +19,10 @@
 $.fn.makeCollapsible = function () {
 
        return this.each(function () {
-               var lpx = 'jquery.makeCollapsible> ';
 
                // Define reused variables and functions
                var $toggle,
+                       lpx = 'jquery.makeCollapsible> ',
                        $that = $(this).addClass( 'mw-collapsible' ), // case: $( '#myAJAXelement' ).makeCollapsible()
                        that = this,
                        collapsetext = $(this).attr( 'data-collapsetext' ),
@@ -230,7 +230,7 @@ $.fn.makeCollapsible = function () {
                                .parent()
                                .prepend( ' [' )
                                .append( '] ' )
-                               .bind( 'click.mw-collapse', function (e) {
+                               .on( 'click.mw-collapse', function ( e ) {
                                        toggleLinkDefault( this, e );
                                } );
 
@@ -252,7 +252,7 @@ $.fn.makeCollapsible = function () {
 
                        // Double check that there is actually a customtoggle link
                        if ( $customTogglers.length ) {
-                               $customTogglers.bind( 'click.mw-collapse', function ( e ) {
+                               $customTogglers.on( 'click.mw-collapse', function ( e ) {
                                        toggleLinkCustom( $(this), e, $that );
                                } );
                        } else {
@@ -279,7 +279,7 @@ $.fn.makeCollapsible = function () {
                                if ( !$toggle.length ) {
                                        $firstRowCells.eq(-1).prepend( $toggleLink );
                                } else {
-                                       $toggleLink = $toggle.unbind( 'click.mw-collapse' ).bind( 'click.mw-collapse', function ( e ) {
+                                       $toggleLink = $toggle.off( 'click.mw-collapse' ).on( 'click.mw-collapse', function ( e ) {
                                                toggleLinkPremade( $toggle, e );
                                        } );
                                }
@@ -300,7 +300,7 @@ $.fn.makeCollapsible = function () {
                                        }
                                        $that.prepend( $toggleLink.wrap( '<li class="mw-collapsible-toggle-li"></li>' ).parent() );
                                } else {
-                                       $toggleLink = $toggle.unbind( 'click.mw-collapse' ).bind( 'click.mw-collapse', function ( e ) {
+                                       $toggleLink = $toggle.off( 'click.mw-collapse' ).on( 'click.mw-collapse', function ( e ) {
                                                toggleLinkPremade( $toggle, e );
                                        } );
                                }
@@ -319,7 +319,7 @@ $.fn.makeCollapsible = function () {
                                if ( !$toggle.length ) {
                                        $that.prepend( $toggleLink );
                                } else {
-                                       $toggleLink = $toggle.unbind( 'click.mw-collapse' ).bind( 'click.mw-collapse', function ( e ) {
+                                       $toggleLink = $toggle.off( 'click.mw-collapse' ).on( 'click.mw-collapse', function ( e ) {
                                                toggleLinkPremade( $toggle, e );
                                        } );
                                }

diff --git a/resources/jquery/jquery.placeholder.js b/resources/jquery/jquery.placeholder.js
index aa356c4..7badb11 100644 (file)
--- a/resources/jquery/jquery.placeholder.js
+++ b/resources/jquery/jquery.placeholder.js
@@ -40,7 +40,7 @@
                                // Hide on focus
                                // Also listen for other events in case $input was
                                // already focused when the events were bound
-                               .bind( 'focus drop keydown paste', function ( e ) {
+                               .on( 'focus drop keydown paste', function ( e ) {
                                        if ( $input.hasClass( 'placeholder' ) ) {
                                                if ( e.type === 'drop' && e.originalEvent.dataTransfer ) {
                                                        // Support for drag&drop. Instead of inserting the dropped

diff --git a/resources/mediawiki.action/mediawiki.action.view.rightClickEdit.js b/resources/mediawiki.action/mediawiki.action.view.rightClickEdit.js
index caf9a9f..d02d432 100644 (file)
--- a/resources/mediawiki.action/mediawiki.action.view.rightClickEdit.js
+++ b/resources/mediawiki.action/mediawiki.action.view.rightClickEdit.js
@@ -1,24 +1,30 @@
 /*
- * JavaScript to enable right click edit functionality
+ * JavaScript to enable right click edit functionality.
+ * When the user right-clicks in a heading, it will open the
+ * edit screen.
  */
-jQuery( function( $ ) {
+jQuery( function ( $ ) {
        // Select all h1-h6 elements that contain editsection links
-       $( 'h1:has(.editsection a), ' +
-               'h2:has(.editsection a), ' +
-               'h3:has(.editsection a), ' +
-               'h4:has(.editsection a), ' +
-               'h5:has(.editsection a), ' +
-               'h6:has(.editsection a)'
-       ).live( 'contextmenu', function( e ) {
-               // Get href of the [edit] link
-               var href = $(this).find( '.editsection a' ).attr( 'href' );
-               // Check if target is the anchor link itself. If so, don't suppress the context menu; this
-               // way the reader can still do things like copy URL, open in new tab etc.
-               var $target = $( e.target );
-               if ( !$target.is( 'a' ) && !$target.parent().is( '.editsection' ) ){
+       // Don't use the ":has:(.editsection a)" selector because it performs very bad.
+       // http://jsperf.com/jq-1-7-2-vs-jq-1-8-1-performance-of-mw-has/2
+       $( document ).on( 'contextmenu', 'h1, h2, h3, h4, h5, h6', function ( e ) {
+               var $edit, href;
+
+               $edit = $( this ).find( '.editsection a' );
+               if ( !$edit.length ) {
+                       return;
+               }
+
+               // Get href of the editsection link
+               href = $edit.prop( 'href' );
+
+               // Headings can contain rich text.
+               // Make sure to not block contextmenu events on (other) anchor tags
+               // inside the heading (e.g. to do things like copy URL, open in new tab, ..).
+               // e.target can be the heading, but it can also be anything inside the heading.
+               if ( href && e.target.nodeName.toLowerCase() !== 'a' ) {
                        window.location = href;
                        e.preventDefault();
-                       return false;
                }
        } );
 } );

diff --git a/resources/mediawiki/mediawiki.htmlform.js b/resources/mediawiki/mediawiki.htmlform.js
index df62105..a4753b9 100644 (file)
--- a/resources/mediawiki/mediawiki.htmlform.js
+++ b/resources/mediawiki/mediawiki.htmlform.js
@@ -31,8 +31,8 @@ $.fn.goOut = function ( instantToggle ) {
 
 /**
  * Bind a function to the jQuery object via live(), and also immediately trigger
- * the function on the objects with an 'instant' paramter set to true
- * @param callback function taking one paramter, which is Bool true when the event
+ * the function on the objects with an 'instant' parameter set to true
+ * @param callback function taking one parameter, which is Bool true when the event
  *     is called immediately, and the EventArgs object when triggered from an event
  */
 $.fn.liveAndTestAtStart = function ( callback ){

diff --git a/skins/common/config.js b/skins/common/config.js
index 23f7302..b1e28ab 100644 (file)
--- a/skins/common/config.js
+++ b/skins/common/config.js
@@ -1,6 +1,17 @@
-(function( $ ) {
-       $( document ).ready( function() {
+( function ( $ ) {
+       $( document ).ready( function () {
+               var $label, labelText;
 
+               function syncText() {
+                       var value = $(this).val()
+                               .replace( /[\[\]\{\}|#<>%+? ]/g, '_' )
+                               .replace( /&/, '&amp;' )
+                               .replace( /__+/g, '_' )
+                               .replace( /^_+/, '' )
+                               .replace( /_+$/, '' );
+                       value = value.substr( 0, 1 ).toUpperCase() + value.substr( 1 );
+                       $label.text( labelText.replace( '$1', value ) );
+               }
 
                // Set up the help system
                $( '.mw-help-field-data' )
@@ -8,7 +19,7 @@
                        .closest( '.mw-help-field-container' )
                                .find( '.mw-help-field-hint' )
                                        .show()
-                                       .click( function() {
+                                       .click( function () {
                                                $(this)
                                                        .closest( '.mw-help-field-container' )
                                                                .find( '.mw-help-field-data' )
@@ -17,22 +28,26 @@
                
                // Show/hide code for DB-specific options
                // FIXME: Do we want slow, fast, or even non-animated (instantaneous) showing/hiding here?
-               $( '.dbRadio' ).each( function() { $( '#' + $(this).attr( 'rel' ) ).hide(); } );
-               $( '#' + $( '.dbRadio:checked' ).attr( 'rel' ) ).show();
-               $( '.dbRadio' ).click( function() {
-                       var $checked = $( '.dbRadio:checked' );
-                       var $wrapper = $( '#' + $checked.attr( 'rel' ) );
-                       if ( !$wrapper.is( ':visible' ) ) {
+               $( '.dbRadio' ).each( function () {
+                       $( document.getElementById( $(this).attr( 'rel' ) ) ).hide();
+               } );
+               $( document.getElementById( $( '.dbRadio:checked' ).attr( 'rel' ) ) ).show();
+               $( '.dbRadio' ).click( function () {
+                       var $checked = $( '.dbRadio:checked' ),
+                               $wrapper = $( document.getElementById( $checked.attr( 'rel' ) ) );
+                       if ( $wrapper.is( ':hidden' ) ) {
                                $( '.dbWrapper' ).hide( 'slow' );
                                $wrapper.show( 'slow' );
                        }
                } );
                
                // Scroll to the bottom of upgrade log
-               $( '#config-live-log' ).find( '> textarea' ).each( function() { this.scrollTop = this.scrollHeight; } );
+               $( '#config-live-log' ).children( 'textarea' ).each( function () {
+                       this.scrollTop = this.scrollHeight;
+               } );
                
                // Show/hide Creative Commons thingy
-               $( '.licenseRadio' ).click( function() {
+               $( '.licenseRadio' ).click( function () {
                        var $wrapper = $( '#config-cc-wrapper' );
                        if ( $( '#config__LicenseCode_cc-choose' ).is( ':checked' ) ) {
                                $wrapper.show( 'slow' );
@@ -42,7 +57,7 @@
                } );
                
                // Show/hide random stuff (email, upload)
-               $( '.showHideRadio' ).click( function() {
+               $( '.showHideRadio' ).click( function () {
                        var $wrapper = $( '#' + $(this).attr( 'rel' ) );
                        if ( $(this).is( ':checked' ) ) {
                                $wrapper.show( 'slow' );
@@ -50,7 +65,7 @@
                                $wrapper.hide( 'slow' );
                        }
                } );
-               $( '.hideShowRadio' ).click( function() {
+               $( '.hideShowRadio' ).click( function () {
                        var $wrapper = $( '#' + $(this).attr( 'rel' ) );
                        if ( $(this).is( ':checked' ) ) {
                                $wrapper.hide( 'slow' );
@@ -64,9 +79,10 @@
                $( '.enabledByOther' ).closest( '.config-block' ).hide();
 
                // Enable/disable "other" textboxes
-               $( '.enableForOther' ).click( function() {
-                       var $textbox = $( '#' + $(this).attr( 'rel' ) );
-                       if ( $(this).val() == 'other' ) { // FIXME: Ugh, this is ugly
+               $( '.enableForOther' ).click( function () {
+                       var $textbox = $( document.getElementById( $(this).attr( 'rel' ) ) );
+                       // FIXME: Ugh, this is ugly
+                       if ( $(this).val() === 'other' ) {
                                $textbox.removeProp( 'readonly' ).closest( '.config-block' ).slideDown( 'fast' );
                        } else {
                                $textbox.prop( 'readonly', true ).closest( '.config-block' ).slideUp( 'fast' );
@@ -77,26 +93,16 @@
                $label = $( 'label[for=config__NamespaceType_site-name]' );
                labelText = $label.text();
                $label.text( labelText.replace( '$1', '' ) );
-               $( '#config_wgSitename' ).bind( 'keyup change', syncText ).each( syncText );
-               function syncText() {
-                       var value = $(this).val()
-                               .replace( /[\[\]\{\}|#<>%+? ]/g, '_' )
-                               .replace( /&/, '&amp;' )
-                               .replace( /__+/g, '_' )
-                               .replace( /^_+/, '' )
-                               .replace( /_+$/, '' );
-                       value = value.substr( 0, 1 ).toUpperCase() + value.substr( 1 );
-                       $label.text( labelText.replace( '$1', value ) );
-               }
+               $( '#config_wgSitename' ).on( 'keyup change', syncText ).each( syncText );
 
                // Show/Hide memcached servers when needed
-               $("input[name$='config_wgMainCacheType']").change( function() {
+               $( 'input[name$="config_wgMainCacheType"]' ).change( function () {
                        var $memc = $( "#config-memcachewrapper" );
-                       if( $( "input[name$='config_wgMainCacheType']:checked" ).val() == 'memcached' ) {
+                       if( $( 'input[name$="config_wgMainCacheType"]:checked' ).val() === 'memcached' ) {
                                $memc.show( 'slow' );
                        } else {
                                $memc.hide( 'slow' );
                        }
                } );
        } );
-})(jQuery);
+}( jQuery ) );