From 2c40fdd4a852fe6ee16feaa3bb6d7d49c7a02606 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Sat, 8 Dec 2012 16:28:10 -0500 Subject: [PATCH] Fix #12600: don't use value property in place of value attribute. Close gh-1063. --- src/attributes.js | 82 ++++++++++++++++++++--------------------- src/support.js | 28 +++++++------- test/unit/attributes.js | 18 +++++---- test/unit/selector.js | 19 ++++++++-- 4 files changed, 79 insertions(+), 68 deletions(-) diff --git a/src/attributes.js b/src/attributes.js index 688282e09..9d603e419 100644 --- a/src/attributes.js +++ b/src/attributes.js @@ -1,4 +1,4 @@ -var nodeHook, boolHook, fixSpecified, +var nodeHook, boolHook, rclass = /[\t\r\n]/g, rreturn = /\r/g, rfocusable = /^(?:input|select|textarea|button|object)$/i, @@ -376,25 +376,6 @@ jQuery.extend({ return value; } } - }, - // Use the value property for back compat - // Use the nodeHook for button elements in IE6/7 (#1954) - value: { - get: function( elem, name ) { - if ( nodeHook && jQuery.nodeName( elem, "button" ) ) { - return nodeHook.get( elem, name ); - } - return name in elem ? - elem.value : - null; - }, - set: function( elem, value, name ) { - if ( nodeHook && jQuery.nodeName( elem, "button" ) ) { - return nodeHook.set( elem, value, name ); - } - // Does not return so that setAttribute is also used - elem.value = value; - } } }, @@ -496,22 +477,39 @@ boolHook = { } }; -// IE6/7 do not support getting/setting some attributes with get/setAttribute -if ( !getSetAttribute ) { +// fix oldIE value attroperty +if ( !getSetAttribute || !jQuery.support.valueAttribute ) { + jQuery.attrHooks.value = { + get: function( elem, name ) { + var ret = elem.getAttributeNode( name ); + return jQuery.nodeName( elem, "input" ) ? - fixSpecified = { - name: true, - id: true, - coords: true + // Ignore the value *property* by using defaultValue + elem.defaultValue : + + ret && ret.specified ? ret.value : undefined; + }, + set: function( elem, value, name ) { + if ( jQuery.nodeName( elem, "input" ) ) { + // Does not return so that setAttribute is also used + elem.defaultValue = value; + } else { + // Use nodeHook if defined (#1954); otherwise setAttribute is fine + return nodeHook && nodeHook.set( elem, value, name ); + } + } }; +} + +// IE6/7 do not support getting/setting some attributes with get/setAttribute +if ( !getSetAttribute ) { // Use this for any attribute in IE6/7 // This fixes almost every IE6/7 issue nodeHook = jQuery.valHooks.button = { get: function( elem, name ) { - var ret; - ret = elem.getAttributeNode( name ); - return ret && ( fixSpecified[ name ] ? ret.value !== "" : ret.specified ) ? + var ret = elem.getAttributeNode( name ); + return ret && ( name === "id" || name === "name" || name === "coords" ? ret.value !== "" : ret.specified ) ? ret.value : undefined; }, @@ -519,8 +517,9 @@ if ( !getSetAttribute ) { // Set the existing or create a new attribute node var ret = elem.getAttributeNode( name ); if ( !ret ) { - ret = elem.ownerDocument.createAttribute( name ); - elem.setAttributeNode( ret ); + elem.setAttributeNode( + (ret = elem.ownerDocument.createAttribute( name )) + ); } ret.value = value += ""; @@ -534,6 +533,15 @@ if ( !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 ) { + nodeHook.set( elem, value === "" ? false : value, name ); + } + }; + // 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 ) { @@ -546,18 +554,6 @@ if ( !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/src/support.js b/src/support.js index 85b6899dc..5c4f192db 100644 --- a/src/support.js +++ b/src/support.js @@ -31,8 +31,11 @@ jQuery.support = (function() { a.style.cssText = "top:1px;float:left;opacity:.5"; support = { + // Test setAttribute on camelCase class. If it works, we need attrFixes when doing get/setAttribute (ie6/7) + getSetAttribute: div.className !== "t", + // IE strips leading whitespace when .innerHTML is used - leadingWhitespace: ( div.firstChild.nodeType === 3 ), + leadingWhitespace: div.firstChild.nodeType === 3, // Make sure that tbody elements aren't automatically inserted // IE will insert them into empty tables @@ -48,7 +51,7 @@ jQuery.support = (function() { // Make sure that URLs aren't manipulated // (IE normalizes it by default) - hrefNormalized: ( a.getAttribute("href") === "/a" ), + hrefNormalized: a.getAttribute("href") === "/a", // Make sure that element opacity exists // (IE uses filter instead) @@ -59,18 +62,13 @@ jQuery.support = (function() { // (IE uses styleFloat instead of cssFloat) cssFloat: !!a.style.cssFloat, - // Make sure that if no value is specified for a checkbox - // that it defaults to "on". - // (WebKit defaults to "" instead) - checkOn: ( input.value === "on" ), + // Check the default checkbox/radio value ("" on WebKit; "on" elsewhere) + checkOn: !!input.value, // Make sure that a selected-by-default option has a working selected property. // (WebKit defaults to false instead of true, IE too, if it's in an optgroup) optSelected: opt.selected, - // Test setAttribute on camelCase class. If it works, we need attrFixes when doing get/setAttribute (ie6/7) - getSetAttribute: div.className !== "t", - // Tests for enctype support on a form (#6743) enctype: !!document.createElement("form").enctype, @@ -79,7 +77,7 @@ jQuery.support = (function() { html5Clone: document.createElement("nav").cloneNode( true ).outerHTML !== "<:nav>", // jQuery.support.boxModel DEPRECATED in 1.8 since we don't support Quirks Mode - boxModel: ( document.compatMode === "CSS1Compat" ), + boxModel: document.compatMode === "CSS1Compat", // Will be defined later submitBubbles: true, @@ -121,16 +119,18 @@ jQuery.support = (function() { div.detachEvent( "onclick", clickFn ); } - // Check if a radio maintains its value - // after being appended to the DOM + // Check if we can trust getAttribute("value") input = document.createElement("input"); + input.setAttribute( "value", "" ); + support.valueAttribute = input.getAttribute( "value" ) === ""; + + // Check if an input maintains its value after becoming a radio input.value = "t"; input.setAttribute( "type", "radio" ); support.radioValue = input.value === "t"; - input.setAttribute( "checked", "checked" ); - // #11217 - WebKit loses check when the name is after the checked attribute + input.setAttribute( "checked", "checked" ); input.setAttribute( "name", "t" ); div.appendChild( input ); diff --git a/test/unit/attributes.js b/test/unit/attributes.js index 4fb3f776e..943d096a8 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -54,7 +54,7 @@ test( "jQuery.propFix integrity test", function() { }); test( "attr(String)", function() { - expect( 48 ); + expect( 50 ); equal( jQuery("#text1").attr("type"), "text", "Check for type attribute" ); equal( jQuery("#radio1").attr("type"), "radio", "Check for type attribute" ); @@ -68,6 +68,7 @@ test( "attr(String)", function() { equal( jQuery("#text1").attr("name"), "action", "Check for name attribute" ); ok( jQuery("#form").attr("action").indexOf("formaction") >= 0, "Check for action attribute" ); equal( jQuery("#text1").attr("value", "t").attr("value"), "t", "Check setting the value attribute" ); + equal( jQuery("#text1").attr("value", "").attr("value"), "", "Check setting the value attribute to empty string" ); equal( jQuery("
").attr("value"), "t", "Check setting custom attr named 'value' on a div" ); equal( jQuery("#form").attr("blah", "blah").attr("blah"), "blah", "Set non-existant attribute on a form" ); equal( jQuery("#foo").attr("height"), undefined, "Non existent height attribute should return undefined" ); @@ -133,9 +134,10 @@ test( "attr(String)", function() { ok( !!~jQuery("#foo").attr("style", "position:absolute;").attr("style").indexOf("position"), "Check style setter" ); // Check value on button element (#1954) - var $button = jQuery("").insertAfter("#button"); - equal( $button.attr("value"), "foobar", "Value retrieval on a button does not return innerHTML" ); - equal( $button.attr("value", "baz").html(), "text", "Setting the value does not change innerHTML" ); + var $button = jQuery("").insertAfter("#button"); + strictEqual( $button.attr("value"), undefined, "Absence of value attribute on a button" ); + equal( $button.attr( "value", "foobar" ).attr("value"), "foobar", "Value attribute on a button does not return innerHTML" ); + equal( $button.attr("value", "baz").html(), "text", "Setting the value attribute does not change innerHTML" ); // Attributes with a colon on a table element (#1591) equal( jQuery("#table").attr("test:attrib"), undefined, "Retrieving a non-existent attribute on a table with a colon does not throw an error." ); @@ -152,7 +154,7 @@ test( "attr(String)", function() { equal( jQuery("
").attr( "title", "something" ).attr("title"), "something", "Set the title attribute." ); ok( jQuery().attr("doesntexist") === undefined, "Make sure undefined is returned when no element is there." ); equal( jQuery("
").attr("value"), undefined, "An unset value on a div returns undefined." ); - equal( jQuery("").attr("value"), "", "An unset value on an input returns current value." ); + strictEqual( jQuery("").attr("value"), undefined, "An unset value on a select returns undefined." ); $form = jQuery("#form").attr( "enctype", "multipart/form-data" ); equal( $form.prop("enctype"), "multipart/form-data", "Set the enctype of a form (encoding in IE6/7 #6743)" ); @@ -196,7 +198,7 @@ test( "attr(String, Function)", function() { equal( jQuery("#text1").attr( "value", function() { return this.id; - })[0].value, + }).attr("value"), "text1", "Set value from id" ); @@ -228,7 +230,7 @@ test( "attr(Hash)", function() { jQuery("#text1").attr({ "value": function() { return this["id"]; - }})[0].value, + }}).attr("value"), "text1", "Set attribute to computed value #1" ); @@ -384,7 +386,7 @@ test( "attr(String, Object)", function() { table.attr("cellspacing", "2"); equal( table[ 0 ]["cellSpacing"], "2", "Check cellspacing is correctly set" ); - equal( jQuery("#area1").attr("value"), "foobar", "Value attribute retrieves the property for backwards compatibility." ); + equal( jQuery("#area1").attr("value"), undefined, "Value attribute is distinct from value property." ); // for #1070 jQuery("#name").attr( "someAttr", "0" ); diff --git a/test/unit/selector.js b/test/unit/selector.js index 57c32cf0d..799603012 100644 --- a/test/unit/selector.js +++ b/test/unit/selector.js @@ -31,15 +31,28 @@ test("class - jQuery only", function() { }); test("attributes - jQuery only", function() { - expect( 2 ); + expect( 4 ); t( "Find elements with a tabindex attribute", "[tabindex]", ["listWithTabIndex", "foodWithNegativeTabIndex", "linkWithTabIndex", "linkWithNegativeTabIndex", "linkWithNoHrefWithTabIndex", "linkWithNoHrefWithNegativeTabIndex"] ); - // jQuery #12523 + + // #12523 deepEqual( jQuery.find( "[title]", null, null, jQuery("#qunit-fixture a").get().concat( document.createTextNode("") ) ), q("google"), "Text nodes fail attribute tests without exception" ); + + // #12600 + ok( + jQuery("") + .prop( "value", "option" ) + .is(":input[value='12600']"), + + ":input[value=foo] selects select by attribute" + ); + ok( jQuery("").prop( "value", "option" ).is(":input[value='12600']"), + ":input[value=foo] selects text input by attribute" + ); }); if ( jQuery.css ) { @@ -74,7 +87,7 @@ test("disconnected nodes", function() { equal( $opt.is(":selected"), true, "selected option" ); var $div = jQuery("
"); - equal( $div.is("div"), true, "Make sure .is('nodeName') works on disconnect nodes." ); + equal( $div.is("div"), true, "Make sure .is('nodeName') works on disconnected nodes." ); }); test("jQuery only - broken", 1, function() { -- 2.39.5