From 0c5becce0e395b89de0f526e857e7ca2717d4ab2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Sat, 15 Jan 2022 01:26:23 +0100 Subject: [PATCH] Widget: Optimize attachment of the _untrackClassesElement listener 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 --- tests/unit/autocomplete/core.js | 23 +++++++++++++++ tests/unit/selectmenu/core.js | 30 +++++++++++++++++++ tests/unit/tabs/core.js | 52 +++++++++++++++++++++++++++++++++ ui/widget.js | 10 +++++-- 4 files changed, 112 insertions(+), 3 deletions(-) diff --git a/tests/unit/autocomplete/core.js b/tests/unit/autocomplete/core.js index a2a36cb4c..2083e16b1 100644 --- a/tests/unit/autocomplete/core.js +++ b/tests/unit/autocomplete/core.js @@ -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" ); +} ); + } ); diff --git a/tests/unit/selectmenu/core.js b/tests/unit/selectmenu/core.js index 05a0e6301..f32cec9b9 100644 --- a/tests/unit/selectmenu/core.js +++ b/tests/unit/selectmenu/core.js @@ -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" ); +} ); + } ); diff --git a/tests/unit/tabs/core.js b/tests/unit/tabs/core.js index 8d4322463..37dd3d4e3 100644 --- a/tests/unit/tabs/core.js +++ b/tests/unit/tabs/core.js @@ -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 = $( + "
  • " + + " " + label + " " + + " Remove Tab" + + "
    • " + ); + var tabContentHtml = "Tab " + tabCounter + " content."; + + element.find( ".ui-tabs-nav" ).append( li ); + element.append( "

    " + tabContentHtml + "

    " ); + 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" ); +} ); + } ); diff --git a/ui/widget.js b/ui/widget.js index 04daaa883..69240f92c 100644 --- a/ui/widget.js +++ b/ui/widget.js @@ -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; @@ -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 ) { -- 2.39.5