Skip to content

Commit

Permalink
Fix #12600: don't use value property in place of value attribute. Close
Browse files Browse the repository at this point in the history 
gh-1063.
  • Loading branch information
@gibson042
gibson042 committed Dec 14, 2012
1 parent 7d61c52 commit 2c40fdd
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 68 deletions.
82 changes: 39 additions & 43 deletions src/attributes.js
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
}
}
},

Expand Down Expand Up @@ -496,31 +477,49 @@ 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;
},
set: function( elem, value, name ) {
// 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 += "";
Expand All @@ -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 ) {
Expand All @@ -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 );
}
};
}


Expand Down
28 changes: 14 additions & 14 deletions src/support.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,

Expand All @@ -79,7 +77,7 @@ jQuery.support = (function() {
html5Clone: document.createElement("nav").cloneNode( true ).outerHTML !== "<:nav></: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,
Expand Down Expand Up @@ -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 );
Expand Down
18 changes: 10 additions & 8 deletions test/unit/attributes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" );
Expand All @@ -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("<div value='t'></div>").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" );
Expand Down Expand Up @@ -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("<button value='foobar'>text</button>").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("<button>text</button>").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." );
Expand All @@ -152,7 +154,7 @@ test( "attr(String)", function() {
equal( jQuery("<div/>").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("<div/>").attr("value"), undefined, "An unset value on a div returns undefined." );
equal( jQuery("<input/>").attr("value"), "", "An unset value on an input returns current value." );
strictEqual( jQuery("<select><option value='property'></option></select>").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)" );
Expand Down Expand Up @@ -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"
);
Expand Down Expand Up @@ -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"
);
Expand Down Expand Up @@ -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" );
Expand Down
19 changes: 16 additions & 3 deletions test/unit/selector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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("<select value='12600'><option value='option' selected='selected'></option><option value=''></option></select>")
.prop( "value", "option" )
.is(":input[value='12600']"),

":input[value=foo] selects select by attribute"
);
ok( jQuery("<input type='text' value='12600'/>").prop( "value", "option" ).is(":input[value='12600']"),
":input[value=foo] selects text input by attribute"
);
});

if ( jQuery.css ) {
Expand Down Expand Up @@ -74,7 +87,7 @@ test("disconnected nodes", function() {
equal( $opt.is(":selected"), true, "selected option" );

var $div = jQuery("<div/>");
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() {
Expand Down

0 comments on commit 2c40fdd

Please sign in to comment.