From a763ae72775f69509d0a8a6b50702bcf2c7e6fbf Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Sun, 9 Dec 2012 00:26:24 -0500 Subject: [PATCH] Fix #11115: Normalize boolean attributes/properties. Close gh-1066. --- src/attributes.js | 83 ++++++++++++++++++------------ src/support.js | 2 +- test/unit/attributes.js | 108 +++++++++++++++++++--------------------- test/unit/selector.js | 12 ++++- test/unit/traversing.js | 6 ++- 5 files changed, 116 insertions(+), 95 deletions(-) diff --git a/src/attributes.js b/src/attributes.js index 9d603e419..bba4efe39 100644 --- a/src/attributes.js +++ b/src/attributes.js @@ -3,8 +3,10 @@ var nodeHook, boolHook, rreturn = /\r/g, rfocusable = /^(?:input|select|textarea|button|object)$/i, rclickable = /^(?:a|area)$/i, - rboolean = /^(?:autofocus|autoplay|async|checked|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped|selected)$/i, - getSetAttribute = jQuery.support.getSetAttribute; + rboolean = /^(?:checked|selected|autofocus|autoplay|async|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped)$/i, + ruseDefault = /^(?:checked|selected)$/i, + getSetAttribute = jQuery.support.getSetAttribute, + getSetInput = jQuery.support.input; jQuery.fn.extend({ attr: function( name, value ) { @@ -338,26 +340,31 @@ jQuery.extend({ }, removeAttr: function( elem, value ) { - var name, propName, isBool, + var name, propName, i = 0, attrNames = value && value.match( core_rnotwhite ); if ( attrNames && elem.nodeType === 1 ) { while ( (name = attrNames[i++]) ) { propName = jQuery.propFix[ name ] || name; - isBool = rboolean.test( name ); + + // Boolean attributes get special treatment (#10870) + if ( rboolean.test( name ) ) { + // Set corresponding property to false for boolean attributes + // Also clear defaultChecked/defaultSelected (if appropriate) for IE<8 + if ( !getSetAttribute && ruseDefault.test( name ) ) { + elem[ jQuery.camelCase( "default-" + name ) ] = + elem[ propName ] = false; + } else { + elem[ propName ] = false; + } // See #9699 for explanation of this approach (setting first, then removal) - // Do not do this for boolean attributes (see #10870) - if ( !isBool ) { + } else { jQuery.attr( elem, name, "" ); } - elem.removeAttribute( getSetAttribute ? name : propName ); - // Set corresponding property to false for boolean attributes - if ( isBool && propName in elem ) { - elem[ propName ] = false; - } + elem.removeAttribute( getSetAttribute ? name : propName ); } } }, @@ -449,36 +456,48 @@ jQuery.extend({ // Hook for boolean attributes boolHook = { get: function( elem, name ) { - // Align boolean attributes with corresponding properties - // Fall back to attribute presence where some booleans are not supported - var attrNode, - property = jQuery.prop( elem, name ); - return property === true || typeof property !== "boolean" && ( attrNode = elem.getAttributeNode(name) ) && attrNode.nodeValue !== false ? + var + // Use .prop to determine if this attribute is understood as boolean + prop = jQuery.prop( elem, name ), + + // Fetch it accordingly + attr = typeof prop === "boolean" && elem.getAttribute( name ), + detail = typeof prop === "boolean" ? + + getSetInput && getSetAttribute ? + attr != null : + // oldIE fabricates an empty string for missing boolean attributes + // and conflates checked/selected into attroperties + ruseDefault.test( name ) ? + elem[ jQuery.camelCase( "default-" + name ) ] : + !!attr : + + // fetch an attribute node for properties not recognized as boolean + elem.getAttributeNode( name ); + + return detail && detail.value !== false ? name.toLowerCase() : undefined; }, set: function( elem, value, name ) { - var propName; if ( value === false ) { // Remove boolean attributes when set to false jQuery.removeAttr( elem, name ); - } else { - // value is true since we know at this point it's type boolean and not false - // Set boolean attributes to the same name and set the DOM property - propName = jQuery.propFix[ name ] || name; - if ( propName in elem ) { - // Only set the IDL specifically if it already exists on the element - elem[ propName ] = true; - } + } else if ( getSetInput && getSetAttribute || !ruseDefault.test( name ) ) { + // IE<8 needs the *property* name + elem.setAttribute( !getSetAttribute && jQuery.propFix[ name ] || name, name ); - elem.setAttribute( name, name.toLowerCase() ); + // Use defaultChecked and defaultSelected for oldIE + } else { + elem[ jQuery.camelCase( "default-" + name ) ] = true; } + return name; } }; // fix oldIE value attroperty -if ( !getSetAttribute || !jQuery.support.valueAttribute ) { +if ( !getSetInput || !getSetAttribute ) { jQuery.attrHooks.value = { get: function( elem, name ) { var ret = elem.getAttributeNode( name ); @@ -524,12 +543,10 @@ if ( !getSetAttribute ) { ret.value = value += ""; - // Break association with cloned elements (#9646) - if ( name !== "value" && value !== elem.getAttribute( name ) ) { - elem.setAttribute( name, value ); - } - - return value; + // Break association with cloned elements by also using setAttribute (#9646) + return name === "value" || value === elem.getAttribute( name ) ? + value : + undefined; } }; diff --git a/src/support.js b/src/support.js index 5c4f192db..70f5f114d 100644 --- a/src/support.js +++ b/src/support.js @@ -122,7 +122,7 @@ jQuery.support = (function() { // Check if we can trust getAttribute("value") input = document.createElement("input"); input.setAttribute( "value", "" ); - support.valueAttribute = input.getAttribute( "value" ) === ""; + support.input = input.getAttribute( "value" ) === ""; // Check if an input maintains its value after becoming a radio input.value = "t"; diff --git a/test/unit/attributes.js b/test/unit/attributes.js index 943d096a8..a4be43aa5 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -123,7 +123,7 @@ test( "attr(String)", function() { optgroup.appendChild( option ); select.appendChild( optgroup ); - equal( jQuery( option ).attr("selected"), "selected", "Make sure that a single option is selected, even when in an optgroup." ); + equal( jQuery( option ).prop("selected"), true, "Make sure that a single option is selected, even when in an optgroup." ); var $img = jQuery("").appendTo("body"); equal( $img.attr("width"), "215", "Retrieve width attribute an an element with display:none." ); @@ -247,13 +247,14 @@ test( "attr(Hash)", function() { }); test( "attr(String, Object)", function() { - expect( 79 ); + expect( 67 ); var div = jQuery("div").attr("foo", "bar"), + i = 0, fail = false; - for ( var i = 0; i < div.size(); i++ ) { - if ( div.get( i ).getAttribute("foo") != "bar" ) { + for ( ; i < div.length; i++ ) { + if ( div[ i ].getAttribute("foo") !== "bar" ) { fail = i; break; } @@ -272,6 +273,7 @@ test( "attr(String, Object)", function() { equal( jQuery("#name").attr("name"), "something", "Set name attribute" ); jQuery("#name").attr( "name", null ); equal( jQuery("#name").attr("name"), undefined, "Remove name attribute" ); + var $input = jQuery( "", { name: "something", id: "specified" @@ -279,66 +281,55 @@ test( "attr(String, Object)", function() { equal( $input.attr("name"), "something", "Check element creation gets/sets the name attribute." ); equal( $input.attr("id"), "specified", "Check element creation gets/sets the id attribute." ); - jQuery("#check2").prop( "checked", true ).prop( "checked", false ).attr( "checked", true ); - equal( document.getElementById("check2").checked, true, "Set checked attribute" ); - equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" ); - equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" ); - jQuery("#check2").attr( "checked", false ); - equal( document.getElementById("check2").checked, false, "Set checked attribute" ); - equal( jQuery("#check2").prop("checked"), false, "Set checked attribute" ); - equal( jQuery("#check2").attr("checked"), undefined, "Set checked attribute" ); - jQuery("#text1").attr( "readonly", true ); - equal( document.getElementById("text1").readOnly, true, "Set readonly attribute" ); - equal( jQuery("#text1").prop("readOnly"), true, "Set readonly attribute" ); - equal( jQuery("#text1").attr("readonly"), "readonly", "Set readonly attribute" ); - jQuery("#text1").attr( "readonly", false ); - equal( document.getElementById("text1").readOnly, false, "Set readonly attribute" ); - equal( jQuery("#text1").prop("readOnly"), false, "Set readonly attribute" ); - equal( jQuery("#text1").attr("readonly"), undefined, "Set readonly attribute" ); - - jQuery("#check2").prop( "checked", true ); - equal( document.getElementById("check2").checked, true, "Set checked attribute" ); - equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" ); - equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" ); - jQuery("#check2").prop( "checked", false ); - equal( document.getElementById("check2").checked, false, "Set checked attribute" ); - equal( jQuery("#check2").prop("checked"), false, "Set checked attribute" ); - equal( jQuery("#check2").attr("checked"), undefined, "Set checked attribute" ); - - jQuery("#check2").attr("checked", "checked"); - equal( document.getElementById("check2").checked, true, "Set checked attribute with 'checked'" ); - equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" ); - equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" ); - - QUnit.reset(); + // As of fixing #11115, we make no promises about the effect of .attr on boolean properties + $input = jQuery("#check2"); + $input.prop( "checked", true ).prop( "checked", false ).attr( "checked", true ); + equal( $input.attr("checked"), "checked", "Set checked (verified by .attr)" ); + $input.prop( "checked", false ).prop( "checked", true ).attr( "checked", false ); + equal( $input.attr("checked"), undefined, "Remove checked (verified by .attr)" ); + + $input = jQuery("#text1").prop( "readOnly", true ).prop( "readOnly", false ).attr( "readonly", true ); + equal( $input.attr("readonly"), "readonly", "Set readonly (verified by .attr)" ); + $input.prop( "readOnly", false ).prop( "readOnly", true ).attr( "readonly", false ); + equal( $input.attr("readonly"), undefined, "Remove readonly (verified by .attr)" ); + + $input = jQuery("#check2").attr( "checked", true ).attr( "checked", false ).prop( "checked", true ); + equal( $input[0].checked, true, "Set checked property (verified by native property)" ); + equal( $input.prop("checked"), true, "Set checked property (verified by .prop)" ); + equal( $input.attr("checked"), undefined, "Setting checked property doesn't affect checked attribute" ); + $input.attr( "checked", false ).attr( "checked", true ).prop( "checked", false ); + equal( $input[0].checked, false, "Clear checked property (verified by native property)" ); + equal( $input.prop("checked"), false, "Clear checked property (verified by .prop)" ); + equal( $input.attr("checked"), "checked", "Clearing checked property doesn't affect checked attribute" ); + + $input = jQuery("#check2").attr( "checked", false ).attr( "checked", "checked" ); + equal( $input.attr("checked"), "checked", "Set checked to 'checked' (verified by .attr)" ); var $radios = jQuery("#checkedtest").find("input[type='radio']"); $radios.eq( 1 ).click(); equal( $radios.eq( 1 ).prop("checked"), true, "Second radio was checked when clicked" ); - equal( $radios.attr("checked"), $radios[ 0 ].checked ? "checked" : undefined, "Known booleans do not fall back to attribute presence (#10278)" ); - - jQuery("#text1").prop( "readOnly", true ); - equal( document.getElementById("text1").readOnly, true, "Set readonly attribute" ); - equal( jQuery("#text1").prop("readOnly"), true, "Set readonly attribute" ); - equal( jQuery("#text1").attr("readonly"), "readonly", "Set readonly attribute" ); - jQuery("#text1").prop( "readOnly", false ); - equal( document.getElementById("text1").readOnly, false, "Set readonly attribute" ); - equal( jQuery("#text1").prop("readOnly"), false, "Set readonly attribute" ); - equal( jQuery("#text1").attr("readonly"), undefined, "Set readonly attribute" ); - - jQuery("#name").attr( "maxlength", "5" ); - equal( document.getElementById("name").maxLength, 5, "Set maxlength attribute" ); - jQuery("#name").attr( "maxLength", "10" ); - equal( document.getElementById("name").maxLength, 10, "Set maxlength attribute" ); + equal( $radios.eq( 0 ).attr("checked"), "checked", "First radio is still [checked]" ); + + $input = jQuery("#text1").attr( "readonly", false ).prop( "readOnly", true ); + equal( $input[0].readOnly, true, "Set readonly property (verified by native property)" ); + equal( $input.prop("readOnly"), true, "Set readonly property (verified by .prop)" ); + $input.attr( "readonly", true ).prop( "readOnly", false ); + equal( $input[0].readOnly, false, "Clear readonly property (verified by native property)" ); + equal( $input.prop("readOnly"), false, "Clear readonly property (verified by .prop)" ); + + $input = jQuery("#name").attr( "maxlength", "5" ); + equal( $input[0].maxLength, 5, "Set maxlength (verified by native property)" ); + $input.attr( "maxLength", "10" ); + equal( $input[0].maxLength, 10, "Set maxlength (verified by native property)" ); // HTML5 boolean attributes var $text = jQuery("#text1").attr({ "autofocus": true, "required": true }); - equal( $text.attr("autofocus"), "autofocus", "Set boolean attributes to the same name" ); - equal( $text.attr( "autofocus", false ).attr("autofocus"), undefined, "Setting autofocus attribute to false removes it" ); - equal( $text.attr("required"), "required", "Set boolean attributes to the same name" ); + equal( $text.attr("autofocus"), "autofocus", "Reading autofocus attribute yields 'autofocus'" ); + equal( $text.attr( "autofocus", false ).attr("autofocus"), undefined, "Setting autofocus to false removes it" ); + equal( $text.attr("required"), "required", "Reading required attribute yields 'required'" ); equal( $text.attr( "required", false ).attr("required"), undefined, "Setting required attribute to false removes it" ); var $details = jQuery("
").appendTo("#qunit-fixture"); @@ -368,13 +359,14 @@ test( "attr(String, Object)", function() { strictEqual( $elem.attr("nonexisting"), undefined, "attr(name, value) works correctly on comment and text nodes (bug #7500)." ); }); + // Register the property name to avoid generating a new global when testing window + Globals.register("nonexisting"); jQuery.each( [ window, document, obj, "#firstp" ], function( i, elem ) { - // use iframeCallback to avoid generating a new global when testing window - var oldVal = elem.iframeCallback, + var oldVal = elem.nonexisting, $elem = jQuery( elem ); strictEqual( $elem.attr("nonexisting"), undefined, "attr works correctly for non existing attributes (bug #7500)." ); - equal( $elem.attr( "iframeCallback", "foo" ).attr("iframeCallback"), "foo", "attr falls back to prop on unsupported arguments" ); - elem.iframeCallback = oldVal; + equal( $elem.attr( "nonexisting", "foo" ).attr("nonexisting"), "foo", "attr falls back to prop on unsupported arguments" ); + elem.nonexisting = oldVal; }); var table = jQuery("#table").append("cellcellcellcellcell"), diff --git a/test/unit/selector.js b/test/unit/selector.js index 799603012..21113207c 100644 --- a/test/unit/selector.js +++ b/test/unit/selector.js @@ -31,7 +31,7 @@ test("class - jQuery only", function() { }); test("attributes - jQuery only", function() { - expect( 4 ); + expect( 6 ); t( "Find elements with a tabindex attribute", "[tabindex]", ["listWithTabIndex", "foodWithNegativeTabIndex", "linkWithTabIndex", "linkWithNegativeTabIndex", "linkWithNoHrefWithTabIndex", "linkWithNoHrefWithNegativeTabIndex"] ); @@ -53,6 +53,14 @@ test("attributes - jQuery only", function() { ok( jQuery("").prop( "value", "option" ).is(":input[value='12600']"), ":input[value=foo] selects text input by attribute" ); + + // #11115 + ok( jQuery("").prop( "checked", false ).is("[checked]"), + "[checked] selects by attribute (positive)" + ); + ok( !jQuery("").prop( "checked", true ).is("[checked]"), + "[checked] selects by attribute (negative)" + ); }); if ( jQuery.css ) { @@ -83,7 +91,7 @@ test("disconnected nodes", function() { var $opt = jQuery("").attr("value", "whipit").appendTo("#qunit-fixture").detach(); equal( $opt.val(), "whipit", "option value" ); equal( $opt.is(":selected"), false, "unselected option" ); - $opt.attr("selected", true); + $opt.prop("selected", true); equal( $opt.is(":selected"), true, "selected option" ); var $div = jQuery("
"); diff --git a/test/unit/traversing.js b/test/unit/traversing.js index fa945365c..94ee061dc 100644 --- a/test/unit/traversing.js +++ b/test/unit/traversing.js @@ -329,7 +329,11 @@ test("not(Selector|undefined)", function() { equal( jQuery("#qunit-fixture > p#ap > a").not("#google").length, 2, "not('selector')" ); deepEqual( jQuery("p").not(".result").get(), q("firstp", "ap", "sndp", "en", "sap", "first"), "not('.class')" ); deepEqual( jQuery("p").not("#ap, #sndp, .result").get(), q("firstp", "en", "sap", "first"), "not('selector, selector')" ); - deepEqual( jQuery("#form option").not("option.emptyopt:contains('Nothing'),[selected],[value='1']").get(), q("option1c", "option1d", "option2c", "option3d", "option3e", "option4e","option5b"), "not('complex selector')"); + deepEqual( + jQuery("#form option").not("option.emptyopt:contains('Nothing'),optgroup *,[value='1']").get(), + q("option1c", "option1d", "option2c", "option2d", "option3c", "option3d", "option3e", "option4d", "option4e", "option5a", "option5b"), + "not('complex selector')" + ); deepEqual( jQuery("#ap *").not("code").get(), q("google", "groups", "anchor1", "mark"), "not('tag selector')" ); deepEqual( jQuery("#ap *").not("code, #mark").get(), q("google", "groups", "anchor1"), "not('tag, ID selector')" ); -- 2.39.5