From ce8d9c0ca59a8f03e119c80ed29c7dbc65efdd85 Mon Sep 17 00:00:00 2001 From: timmywil Date: Thu, 6 Oct 2011 17:17:51 -0400 Subject: [PATCH] Add a hook for removing contenteditable in IE6/7 and remove the unnecessary jQuery.attrFix. Fixes #10429. --- src/attributes.js | 43 +++++++++++++++++++---------------------- test/unit/attributes.js | 20 ++++++++++--------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/attributes.js b/src/attributes.js index 71353cf78..345845689 100644 --- a/src/attributes.js +++ b/src/attributes.js @@ -291,11 +291,6 @@ jQuery.extend({ offset: true }, - attrFix: { - // Always normalize to ensure hook usage - tabindex: "tabIndex" - }, - attr: function( elem, name, value, pass ) { var nType = elem.nodeType; @@ -318,20 +313,8 @@ jQuery.extend({ // Normalize the name if needed if ( notxml ) { - name = jQuery.attrFix[ name ] || name; - - hooks = jQuery.attrHooks[ name ]; - - if ( !hooks ) { - // Use boolHook for boolean attributes - if ( rboolean.test( name ) ) { - hooks = boolHook; - - // Use nodeHook if available( IE6/7 ) - } else if ( nodeHook ) { - hooks = nodeHook; - } - } + name = name.toLowerCase(); + hooks = jQuery.attrHooks[ name ] || (rboolean.test( name ) ? boolHook : nodeHook); } if ( value !== undefined ) { @@ -371,8 +354,7 @@ jQuery.extend({ l = attrNames.length; for ( ; i < l; i++ ) { - name = attrNames[ i ]; - name = jQuery.attrFix[ name ] || name; + name = attrNames[ i ].toLowerCase(); // See #9699 for explanation of this approach (setting first, then removal) jQuery.attr( elem, name, "" ); @@ -493,8 +475,8 @@ jQuery.extend({ } }); -// Add the tabindex propHook to attrHooks for back-compat -jQuery.attrHooks.tabIndex = jQuery.propHooks.tabIndex; +// Add the tabIndex propHook to attrHooks for back-compat (different case is intentional) +jQuery.attrHooks.tabindex = jQuery.propHooks.tabIndex; // Hook for boolean attributes boolHook = { @@ -556,6 +538,9 @@ if ( !jQuery.support.getSetAttribute ) { } }; + // Apply the nodeHook to tabindex + jQuery.attrHooks.tabindex.set = nodeHook.set; + // Set width and height to auto instead of 0 on empty string( Bug #8150 ) // This is for removals jQuery.each([ "width", "height" ], function( i, name ) { @@ -568,6 +553,18 @@ if ( !jQuery.support.getSetAttribute ) { } }); }); + + // Set contenteditable to false on removals(#10429) + // Setting to empty string throws an error as an invalid value + jQuery.attrHooks.contenteditable = { + get: nodeHook.get, + set: function( elem, value, name ) { + if ( value === "" ) { + value = "false"; + } + nodeHook.set( elem, value, name ); + } + }; } diff --git a/test/unit/attributes.js b/test/unit/attributes.js index da39933d3..fd97d1c05 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -4,8 +4,8 @@ var bareObj = function(value) { return value; }; var functionReturningObj = function(value) { return (function() { return value; }); }; -test("jQuery.attrFix/jQuery.propFix integrity test", function() { - expect(2); +test("jQuery.propFix integrity test", function() { + expect(1); // This must be maintained and equal jQuery.attrFix when appropriate // Ensure that accidental or erroneous property @@ -26,11 +26,6 @@ test("jQuery.attrFix/jQuery.propFix integrity test", function() { contenteditable: "contentEditable" }; - var propsShouldBe = { - tabindex: "tabIndex" - }; - - deepEqual(propsShouldBe, jQuery.attrFix, "jQuery.attrFix passes integrity check"); deepEqual(props, jQuery.propFix, "jQuery.propFix passes integrity check"); }); @@ -456,7 +451,7 @@ test("attr('tabindex', value)", function() { }); test("removeAttr(String)", function() { - expect(8); + expect(9); equal( jQuery("#mark").removeAttr( "class" )[0].className, "", "remove class" ); equal( jQuery("#form").removeAttr("id").attr("id"), undefined, "Remove id" ); equal( jQuery("#foo").attr("style", "position:absolute;").removeAttr("style").attr("style"), undefined, "Check removing style attribute" ); @@ -468,6 +463,13 @@ test("removeAttr(String)", function() { equal( document.getElementById("check1").checked, false, "removeAttr sets boolean properties to false" ); jQuery("#text1").prop("readOnly", true).removeAttr("readonly"); equal( document.getElementById("text1").readOnly, false, "removeAttr sets boolean properties to false" ); + + try { + jQuery("#first").attr("contenteditable", "true").removeAttr("contenteditable"); + ok( true, "Removing contenteditable does not throw an error."); + } catch(e) { + ok( false, "Removing contenteditable threw an error (#10429)" ); + } }); test("prop(String, Object)", function() { @@ -882,7 +884,7 @@ var testAddClass = function(valueObj) { equals( div.attr("class"), "foo bar baz", "Make sure there isn't too much trimming." ); div.removeClass(); - div.addClass( valueObj("foo") ).addClass( valueObj("foo") ) + div.addClass( valueObj("foo") ).addClass( valueObj("foo") ); equal( div.attr("class"), "foo", "Do not add the same class twice in separate calls." ); div.addClass( valueObj("fo") ); -- 2.39.5