From 3491515f77af036461ae69c2a1f0f524933f0cae Mon Sep 17 00:00:00 2001 From: Alexander Schmitz Date: Mon, 12 Sep 2016 10:26:24 -0400 Subject: [PATCH 1/3] Widget: Untrack classes elements when they are removed from the DOM Fixes #15043 --- tests/unit/widget/classes.js | 42 +++++++++++++++++++++++++++++++----- ui/widget.js | 10 +++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/tests/unit/widget/classes.js b/tests/unit/widget/classes.js index d355c3a3632..b52f2424d7d 100644 --- a/tests/unit/widget/classes.js +++ b/tests/unit/widget/classes.js @@ -15,6 +15,8 @@ QUnit.module( "widget factory classes", { }, _create: function() { this.span = $( "" ) + .add( "" ) + .add( "" ) .appendTo( this.element ); this.element.wrap( "
" ); @@ -68,9 +70,9 @@ function elementHasClasses( widget, method, assert ) { "_" + method + "Class works with ( null, extra" + toggle + " )" ); assert.hasClasses( widget.parent(), "ui-classes-widget ui-theme-widget", "_" + method + "Class works with ( element, null, extra" + toggle + " )" ); - assert.hasClasses( widget.find( "span" ), "ui-classes-span ui-core-span", + assert.hasClasses( widget.find( "span" )[ 0 ], "ui-classes-span ui-core-span", "_" + method + "Class works with ( element, keys, extra" + toggle + " )" ); - assert.hasClasses( widget.find( "span" ), "ui-core-span-null", + assert.hasClasses( widget.find( "span" )[ 0 ], "ui-core-span-null", "_" + method + "Class works with ( element, keys, null" + toggle + " )" ); } function elementLacksClasses( widget, method, assert ) { @@ -84,9 +86,9 @@ function elementLacksClasses( widget, method, assert ) { "_" + method + "Class works with ( null, extra" + toggle + " )" ); assert.lacksClasses( widget.parent(), "ui-classes-widget ui-theme-widget", "_" + method + "Class works with ( element, null, extra" + toggle + " )" ); - assert.lacksClasses( widget.find( "span" ), "ui-classes-span ui-core-span", + assert.lacksClasses( widget.find( "span" )[ 0 ], "ui-classes-span ui-core-span", "_" + method + "Class works with ( element, keys, extra" + toggle + " )" ); - assert.lacksClasses( widget.find( "span" ), "ui-core-span-null", + assert.lacksClasses( widget.find( "span" )[ 0 ], "ui-core-span-null", "_" + method + "Class works with ( element, keys, null" + toggle + " )" ); } @@ -113,7 +115,7 @@ QUnit.test( ".option() - classes setter", function( assert ) { "Setting to empty value leaves structure class" ); assert.lacksClasses( testWidget.element, "ui-theme-element-2", "Setting empty value removes previous value classes" ); - assert.hasClasses( testWidget.span, "ui-classes-span custom-theme-span", + assert.hasClasses( testWidget.span[ 0 ], "ui-classes-span custom-theme-span", "Adding a class to an empty value works as expected" ); assert.hasClasses( testWidget.wrapper, "ui-classes-widget custom-theme-widget", "Appending a class to the current value works as expected" ); @@ -144,4 +146,34 @@ QUnit.test( "._add/_remove/_toggleClass()", function( assert ) { elementLacksClasses( widget, "remove", assert ); } ); +QUnit.test( "Classes elements are untracked as they are removed from the dom", function( assert ) { + assert.expect( 9 ); + + var widget = $( "#widget" ).classesWidget(); + var instance = widget.classesWidget( "instance" ); + + assert.equal( 3, instance.classesElementLookup[ "ui-classes-span" ].length, + "Widget is tracking 3 ui-classes-span elements" ); + assert.equal( 3, instance.classesElementLookup[ "ui-core-span-null" ].length, + "Widget is tracking 3 ui-core-span-null elements" ); + assert.equal( 3, instance.classesElementLookup[ "ui-core-span" ].length, + "Widget is tracking 3 ui-core-span elements" ); + + widget.find( "span" ).eq( 0 ).remove(); + assert.equal( 2, instance.classesElementLookup[ "ui-classes-span" ].length, + "After removing 1 span from dom 2 ui-classes-span elements are tracked" ); + assert.equal( 2, instance.classesElementLookup[ "ui-core-span-null" ].length, + "After removing 1 span from dom 2 ui-core-span-null elements are tracked" ); + assert.equal( 2, instance.classesElementLookup[ "ui-core-span" ].length, + "After removing 1 span from dom 2 ui-core-span elements are tracked" ); + + widget.find( "span" ).remove(); + assert.equal( 0, instance.classesElementLookup[ "ui-classes-span" ].length, + "No ui-classes-span elements are tracked after removing all spans" ); + assert.equal( 0, instance.classesElementLookup[ "ui-core-span-null" ].length, + "No ui-core-span-null elements are tracked after removing all spans" ); + assert.equal( 0, instance.classesElementLookup[ "ui-core-span" ].length, + "No ui-core-span elements are tracked after removing all spans" ); +} ); + } ); diff --git a/ui/widget.js b/ui/widget.js index 06a3ce88e27..60ee0296bd7 100644 --- a/ui/widget.js +++ b/ui/widget.js @@ -514,6 +514,16 @@ $.Widget.prototype = { } } + this._on( options.element, { + "remove": function( event ) { + $.each( that.classesElementLookup, function( key, value ) { + if ( $.inArray( event.target ) ) { + that.classesElementLookup[ key ] = $( value.not( event.target ).get() ); + } + } ); + } + } ); + if ( options.keys ) { processClassString( options.keys.match( /\S+/g ) || [], true ); } From cf7d04ff872551fc57d7ed2b4be64f7aed5d4dd6 Mon Sep 17 00:00:00 2001 From: Alexander Schmitz Date: Mon, 12 Sep 2016 12:09:12 -0400 Subject: [PATCH 2/3] Widget: Fixup based on review comments --- tests/unit/widget/classes.js | 20 ++++++++++---------- ui/widget.js | 17 ++++++++++------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/tests/unit/widget/classes.js b/tests/unit/widget/classes.js index b52f2424d7d..28f61f272e6 100644 --- a/tests/unit/widget/classes.js +++ b/tests/unit/widget/classes.js @@ -146,33 +146,33 @@ QUnit.test( "._add/_remove/_toggleClass()", function( assert ) { elementLacksClasses( widget, "remove", assert ); } ); -QUnit.test( "Classes elements are untracked as they are removed from the dom", function( assert ) { +QUnit.test( "Classes elements are untracked as they are removed from the DOM", function( assert ) { assert.expect( 9 ); var widget = $( "#widget" ).classesWidget(); var instance = widget.classesWidget( "instance" ); - assert.equal( 3, instance.classesElementLookup[ "ui-classes-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 3, "Widget is tracking 3 ui-classes-span elements" ); - assert.equal( 3, instance.classesElementLookup[ "ui-core-span-null" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 3, "Widget is tracking 3 ui-core-span-null elements" ); - assert.equal( 3, instance.classesElementLookup[ "ui-core-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 3, "Widget is tracking 3 ui-core-span elements" ); widget.find( "span" ).eq( 0 ).remove(); - assert.equal( 2, instance.classesElementLookup[ "ui-classes-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 2, "After removing 1 span from dom 2 ui-classes-span elements are tracked" ); - assert.equal( 2, instance.classesElementLookup[ "ui-core-span-null" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 2, "After removing 1 span from dom 2 ui-core-span-null elements are tracked" ); - assert.equal( 2, instance.classesElementLookup[ "ui-core-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 2, "After removing 1 span from dom 2 ui-core-span elements are tracked" ); widget.find( "span" ).remove(); - assert.equal( 0, instance.classesElementLookup[ "ui-classes-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-classes-span" ].length, 0, "No ui-classes-span elements are tracked after removing all spans" ); - assert.equal( 0, instance.classesElementLookup[ "ui-core-span-null" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span-null" ].length, 0, "No ui-core-span-null elements are tracked after removing all spans" ); - assert.equal( 0, instance.classesElementLookup[ "ui-core-span" ].length, + assert.equal( instance.classesElementLookup[ "ui-core-span" ].length, 0, "No ui-core-span elements are tracked after removing all spans" ); } ); diff --git a/ui/widget.js b/ui/widget.js index 60ee0296bd7..f00c095e024 100644 --- a/ui/widget.js +++ b/ui/widget.js @@ -515,13 +515,7 @@ $.Widget.prototype = { } this._on( options.element, { - "remove": function( event ) { - $.each( that.classesElementLookup, function( key, value ) { - if ( $.inArray( event.target ) ) { - that.classesElementLookup[ key ] = $( value.not( event.target ).get() ); - } - } ); - } + "remove": "_untrackClassesElement" } ); if ( options.keys ) { @@ -534,6 +528,15 @@ $.Widget.prototype = { return full.join( " " ); }, + _untrackClassesElement: function( event ) { + var that = this; + $.each( that.classesElementLookup, function( key, value ) { + if ( $.inArray( event.target, that.classesElementLookup[ key ] ) !== -1 ) { + that.classesElementLookup[ key ] = $( value.not( event.target ).get() ); + } + } ); + }, + _removeClass: function( element, keys, extra ) { return this._toggleClass( element, keys, extra, false ); }, From bdfc1e5a19a94b3c0ba4b309b283c540a4b98d62 Mon Sep 17 00:00:00 2001 From: Alexander Schmitz Date: Tue, 13 Sep 2016 09:40:13 -0400 Subject: [PATCH 3/3] Widget: Fixup --- ui/widget.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/widget.js b/ui/widget.js index f00c095e024..a3922675f1d 100644 --- a/ui/widget.js +++ b/ui/widget.js @@ -531,7 +531,7 @@ $.Widget.prototype = { _untrackClassesElement: function( event ) { var that = this; $.each( that.classesElementLookup, function( key, value ) { - if ( $.inArray( event.target, that.classesElementLookup[ key ] ) !== -1 ) { + if ( $.inArray( event.target, value ) !== -1 ) { that.classesElementLookup[ key ] = $( value.not( event.target ).get() ); } } );