From c9ba56de4883800f7ae937351e59aaf2d97f1623 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 4 Sep 2012 21:47:20 +0200 Subject: [PATCH] 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 --- resources/jquery/jquery.makeCollapsible.js | 12 ++-- resources/jquery/jquery.placeholder.js | 2 +- .../mediawiki.action.view.rightClickEdit.js | 38 ++++++----- resources/mediawiki/mediawiki.htmlform.js | 4 +- skins/common/config.js | 66 ++++++++++--------- 5 files changed, 67 insertions(+), 55 deletions(-) diff --git a/resources/jquery/jquery.makeCollapsible.js b/resources/jquery/jquery.makeCollapsible.js index 33f875210b..0a4d3645b9 100644 --- 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( '
  • ' ).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 aa356c4c50..7badb11ae8 100644 --- 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 caf9a9f2e0..d02d43273e 100644 --- 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 df62105944..a4753b992c 100644 --- 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 23f7302cb8..b1e28aba37 100644 --- 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( /&/, '&' ) + .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( /&/, '&' ) - .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 ) ); -- 2.20.1