Data: use removeAttribute in cleanData to bypass Chrome bug

Related: https://code.google.com/p/chromium/issues/detail?id=378607 Fixes gh-1664
jquery · Dec 10, 2014 · 9d1d90e · 9d1d90e
1 parent 57652ee
commit 9d1d90e
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 41 deletions.
diff --git a/src/data.js b/src/data.js
@@ -230,9 +230,9 @@ function internalRemoveData( elem, name, pvt ) {
 		/* jshint eqeqeq: true */
 		delete cache[ id ];
 
-	// When all else fails, null
+	// When all else fails, undefined
 	} else {
-		cache[ id ] = null;
+		cache[ id ] = undefined;
 	}
 }
 

diff --git a/src/data/support.js b/src/data/support.js
@@ -5,15 +5,12 @@ define([
 (function() {
 	var div = document.createElement( "div" );
 
-	// Execute the test only if not already executed in another module.
+	// Support: IE<9
-	if (support.deleteExpando == null) {
+	support.deleteExpando = true;
-		// Support: IE<9
+	try {
-		support.deleteExpando = true;
+		delete div.test;
-		try {
+	} catch ( e ) {
-			delete div.test;
+		support.deleteExpando = false;
-		} catch ( e ) {
-			support.deleteExpando = false;
-		}
 	}
 
 	// Null elements to avoid leaks in IE.

diff --git a/src/manipulation.js b/src/manipulation.js
@@ -367,7 +367,7 @@ jQuery.extend({
 			i = 0,
 			internalKey = jQuery.expando,
 			cache = jQuery.cache,
-			deleteExpando = support.deleteExpando,
+			attributes = support.attributes,
 			special = jQuery.event.special;
 
 		for ( ; (elem = elems[i]) != null; i++ ) {
@@ -394,17 +394,18 @@ jQuery.extend({
 
 						delete cache[ id ];
 
-						// IE does not allow us to delete expando properties from nodes,
+						// Support: IE<9
-						// nor does it have a removeAttribute function on Document nodes;
+						// IE does not allow us to delete expando properties from nodes
-						// we must handle all of these cases
+						// IE creates expando attributes along with the property
-						if ( deleteExpando ) {
+						// IE does not have a removeAttribute function on Document nodes
-							delete elem[ internalKey ];
+						if ( !attributes && typeof elem.removeAttribute !== "undefined" ) {
-
-						} else if ( typeof elem.removeAttribute !== "undefined" ) {
 							elem.removeAttribute( internalKey );
 
+						// Webkit & Blink performance suffers when deleting properties
+						// from DOM nodes, so set to undefined instead
+						// https://code.google.com/p/chromium/issues/detail?id=378607
 						} else {
-							elem[ internalKey ] = null;
+							elem[ internalKey ] = undefined;
 						}
 
 						deletedIds.push( id );

diff --git a/src/manipulation/support.js b/src/manipulation/support.js
@@ -1,6 +1,7 @@
 define([
+	"../core",
 	"../var/support"
-], function( support ) {
+], function( jQuery, support ) {
 
 (function() {
 	var div = document.createElement( "div" ),
@@ -38,16 +39,11 @@ define([
 	// Cloned elements keep attachEvent handlers, we use addEventListener on IE9+
 	support.noCloneEvent = !!div.addEventListener;
 
-	// Execute the test only if not already executed in another module.
+	// Support: IE<9
-	if (support.deleteExpando == null) {
+	// Since attributes and properties are the same in IE,
-		// Support: IE<9
+	// cleanData must set properties to undefined rather than use removeAttribute
-		support.deleteExpando = true;
+	div[ jQuery.expando ] = 1;
-		try {
+	support.attributes = !div.getAttribute( jQuery.expando );
-			delete div.test;
-		} catch ( e ) {
-			support.deleteExpando = false;
-		}
-	}
 })();
 
 return support;

diff --git a/test/unit/data.js b/test/unit/data.js
@@ -100,18 +100,10 @@ test("jQuery.data(document)", 25, function() {
 });
 
 test("Expando cleanup", 4, function() {
-	var expected, actual,
+	var div = document.createElement("div");
-		div = document.createElement("div");
 
 	function assertExpandoAbsent(message) {
-		if (jQuery.support.deleteExpando) {
+		strictEqual( div[ jQuery.expando ], undefined, message );
-			expected = false;
-			actual = jQuery.expando in div;
-		} else {
-			expected = null;
-			actual = div[ jQuery.expando ];
-		}
-		equal( actual, expected, message );
 	}
 
 	assertExpandoAbsent("There is no expando on new elements");

diff --git a/test/unit/support.js b/test/unit/support.js
@@ -73,6 +73,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 		// Android browser on Android >= 4.4).
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -103,6 +104,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /(msie 10\.0|trident\/7\.0)/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": false,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -133,6 +135,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /msie 9\.0/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": false,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -163,6 +166,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /msie 8\.0/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": false,
 			"boxSizingReliable": false,
 			"changeBubbles": false,
 			"checkClone": true,
@@ -193,6 +197,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /(6|7)\.0(\.\d+|) safari/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -223,6 +228,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /8.0(\.\d+|) safari/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -253,6 +259,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /firefox/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -283,6 +290,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /iphone os/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,
@@ -313,6 +321,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /android 4\.[0-3]/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": false,
@@ -343,6 +352,7 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
 	} else if ( /android 2\.3/i.test( userAgent ) ) {
 		expected = {
 			"ajax": true,
+			"attributes": true,
 			"boxSizingReliable": true,
 			"changeBubbles": true,
 			"checkClone": true,