Skip to content

Commit

Permalink
Widget: Optimize attachment of the _untrackClassesElement listener
Browse files Browse the repository at this point in the history
jQuery UI 1.13.0 changed the logic attaching the `_untrackClassesElement`
listener in the `_classes` widget method; one of the side effects was calling
`this._on` for each node that needed the listener. That caused a severe
performance degradation for large comboboxes as each `_on` jQuery UI call
causes a jQuery `add` call that calls Sizzle's `uniqueSort` underneath.

Instead, collect the nodes that need the listener and then, outside of the loop,
create a jQuery object out of them and attach the listener once. That's still
slower than the jQuery 1.12 version but only slightly: 936 ms to 1.03s on a very
large list on a recent MacBook Pro, compared to ~30 seconds before this patch.

Fixes gh-2014
Closes gh-2037
  • Loading branch information
mgol authored Jan 15, 2022
1 parent 4a7cec3 commit 0c5becc
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 3 deletions.
23 changes: 23 additions & 0 deletions tests/unit/autocomplete/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,4 +443,27 @@ QUnit.test( "Close on click outside when focus remains", function( assert ) {
} );
} );

QUnit.test( "extra listeners created during typing (trac-15082, trac-15095)", function( assert ) {
assert.expect( 2 );

var origRemoveListenersCount;
var element = $( "#autocomplete" ).autocomplete( {
source: [ "java", "javascript" ],
delay: 0
} );

element.val( "j" ).autocomplete( "search", "j" );
origRemoveListenersCount = jQuery._data( element[ 0 ], "events" ).remove.length;

element.val( "ja" ).autocomplete( "search", "ja" );
assert.equal( jQuery._data( element[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after typing multiple letters" );

element.val( "jav" ).autocomplete( "search", "jav" );
assert.equal( jQuery._data( element[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after typing multiple letters" );
} );

} );
30 changes: 30 additions & 0 deletions tests/unit/selectmenu/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,4 +404,34 @@ QUnit.test( "Options with hidden attribute should not be rendered", function( as
} );
} );

QUnit.test( "extra listeners created after selection (trac-15078, trac-15152)", function( assert ) {
assert.expect( 3 );

var origRemoveListenersCount;
var element = $( "#speed" ).selectmenu();
var menu = element.selectmenu( "widget" );

element.val( "Slow" );
element.selectmenu( "refresh" );
origRemoveListenersCount = jQuery._data( menu[ 0 ], "events" ).remove.length;

element.val( "Fast" );
element.selectmenu( "refresh" );
assert.equal( jQuery._data( menu[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after typing multiple letters" );

element.val( "Faster" );
element.selectmenu( "refresh" );
assert.equal( jQuery._data( menu[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after typing multiple letters" );

element.val( "Slow" );
element.selectmenu( "refresh" );
assert.equal( jQuery._data( menu[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after typing multiple letters" );
} );

} );
52 changes: 52 additions & 0 deletions tests/unit/tabs/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -675,4 +675,56 @@ QUnit.test( "#4033 - IE expands hash to full url and misinterprets tab as ajax",
state( assert, element, 1 );
} );


QUnit.test( "extra listeners created when tabs are added/removed (trac-15136)", function( assert ) {
assert.expect( 3 );

var origRemoveListenersCount;
var element = $( "#tabs1" ).tabs();
var tabCounter = 10;

function addTab() {
var label = "Tab " + tabCounter;
var id = "tabs-" + tabCounter;
var li = $(
"<li>" +
" <a href='#" + id + "'>" + label + "</a> " +
" <span class='ui-icon ui-icon-close' role='presentation'>Remove Tab</span>" +
"</li>"
);
var tabContentHtml = "Tab " + tabCounter + " content.";

element.find( ".ui-tabs-nav" ).append( li );
element.append( "<div id='" + id + "'><p>" + tabContentHtml + "</p></div>" );
element.tabs( "refresh" );
tabCounter++;
}

function removeLastTab() {
element.find( ".ui-icon-close" ).last().trigger( "click" );
}

origRemoveListenersCount = jQuery._data( element[ 0 ], "events" ).remove.length;

addTab();
assert.equal( jQuery._data( element[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after adding a new tab" );

addTab();
addTab();
addTab();
assert.equal( jQuery._data( element[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after adding multiple tabs" );

removeLastTab();
removeLastTab();
removeLastTab();
removeLastTab();
assert.equal( jQuery._data( element[ 0 ], "events" ).remove.length,
origRemoveListenersCount,
"No extra listeners after removing all the extra tabs" );
} );

} );
10 changes: 7 additions & 3 deletions ui/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,8 @@ $.Widget.prototype = {
}, options );

function bindRemoveEvent() {
var nodesToBind = [];

options.element.each( function( _, element ) {
var isTracked = $.map( that.classesElementLookup, function( elements ) {
return elements;
Expand All @@ -508,11 +510,13 @@ $.Widget.prototype = {
} );

if ( !isTracked ) {
that._on( $( element ), {
remove: "_untrackClassesElement"
} );
nodesToBind.push( element );
}
} );

that._on( $( nodesToBind ), {
remove: "_untrackClassesElement"
} );
}

function processClassString( classes, checkOption ) {
Expand Down

0 comments on commit 0c5becc

Please sign in to comment.