Attributes: Don't warn against removeAttr if property false

Some frameworks, like AngularJS, invoke `removeAttr` on boolean attributes but only after setting the respective property to `false`. Such usage is not dangerous and would continue to work even after Migrate is removed. Therefore, only warn for `removeAttr(booleanAttribute)` if the property wasn't `false` already. Fixes gh-471 Closes gh-484
jquery · Feb 23, 2023 · 072e44d · 072e44d
1 parent 0d082f9
commit 072e44d
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/src/jquery/attributes.js b/src/jquery/attributes.js
@@ -5,10 +5,24 @@ var oldRemoveAttr = jQuery.fn.removeAttr,
 	rmatchNonSpace = /\S+/g;
 
 migratePatchFunc( jQuery.fn, "removeAttr", function( name ) {
-	var self = this;
+	var self = this,
+		patchNeeded = false;
 
 	jQuery.each( name.match( rmatchNonSpace ), function( _i, attr ) {
 		if ( jQuery.expr.match.bool.test( attr ) ) {
+
+			// Only warn if at least a single node had the property set to
+			// something else than `false`. Otherwise, this Migrate patch
+			// doesn't influence the behavior and there's no need to set or warn.
+			self.each( function() {
+				if ( jQuery( this ).prop( attr ) !== false ) {
+					patchNeeded = true;
+					return false;
+				}
+			} );
+		}
+
+		if ( patchNeeded ) {
 			migrateWarn( "removeAttr-bool",
 				"jQuery.fn.removeAttr no longer sets boolean properties: " + attr );
 			self.prop( attr, false );

diff --git a/test/unit/jquery/attributes.js b/test/unit/jquery/attributes.js
@@ -2,7 +2,7 @@
 QUnit.module( "attributes" );
 
 QUnit.test( ".removeAttr( boolean attribute )", function( assert ) {
-	assert.expect( 8 );
+	assert.expect( 14 );
 
 	expectNoWarning( assert, "non-boolean attr", function() {
 		var $div = jQuery( "<div />" )
@@ -40,6 +40,26 @@ QUnit.test( ".removeAttr( boolean attribute )", function( assert ) {
 			.removeAttr( "size" );
 	} );
 
+	expectNoWarning( assert, "boolean attr when prop false", function() {
+		var $inp = jQuery( "<input type=checkbox/>" )
+				.attr( "checked", "checked" )
+				.prop( "checked", false )
+				.removeAttr( "checked" );
+
+		assert.equal( $inp.attr( "checked" ), null, "boolean attribute was removed" );
+		assert.equal( $inp.prop( "checked" ), false, "property was not changed" );
+	} );
+
+	expectWarning( assert, "boolean attr when only some props false", 1, function() {
+		var $inp = jQuery( "<input type=checkbox/><input type=checkbox/><input type=checkbox/>" )
+				.attr( "checked", "checked" )
+				.prop( "checked", false )
+				.eq( 1 ).prop( "checked", true ).end()
+				.removeAttr( "checked" );
+
+		assert.equal( $inp.attr( "checked" ), null, "boolean attribute was removed" );
+		assert.equal( $inp.eq( 1 ).prop( "checked" ), false, "property was changed" );
+	} );
 } );
 
 QUnit.test( ".toggleClass( boolean )", function( assert ) {