Skip to content

Commit a763ae7

Browse files
gibson042gibson042
committed
Fix #11115: Normalize boolean attributes/properties. Close jquerygh-1066.
1 parent 55a8ba5 commit a763ae7

File tree

5 files changed

+116
-95
lines changed

5 files changed

+116
-95
lines changed

‎src/attributes.js

Lines changed: 50 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ var nodeHook, boolHook,
33
rreturn = /\r/g,
44
rfocusable = /^(?:input|select|textarea|button|object)$/i,
55
rclickable = /^(?:a|area)$/i,
6-
rboolean = /^(?:autofocus|autoplay|async|checked|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped|selected)$/i,
7-
getSetAttribute = jQuery.support.getSetAttribute;
6+
rboolean = /^(?:checked|selected|autofocus|autoplay|async|controls|defer|disabled|hidden|loop|multiple|open|readonly|required|scoped)$/i,
7+
ruseDefault = /^(?:checked|selected)$/i,
8+
getSetAttribute = jQuery.support.getSetAttribute,
9+
getSetInput = jQuery.support.input;
810

911
jQuery.fn.extend({
1012
attr: function( name, value ) {
@@ -338,26 +340,31 @@ jQuery.extend({
338340
},
339341

340342
removeAttr: function( elem, value ) {
341-
var name, propName, isBool,
343+
var name, propName,
342344
i = 0,
343345
attrNames = value && value.match( core_rnotwhite );
344346

345347
if ( attrNames && elem.nodeType === 1 ) {
346348
while ( (name = attrNames[i++]) ) {
347349
propName = jQuery.propFix[ name ] || name;
348-
isBool = rboolean.test( name );
350+
351+
// Boolean attributes get special treatment (#10870)
352+
if ( rboolean.test( name ) ) {
353+
// Set corresponding property to false for boolean attributes
354+
// Also clear defaultChecked/defaultSelected (if appropriate) for IE<8
355+
if ( !getSetAttribute && ruseDefault.test( name ) ) {
356+
elem[ jQuery.camelCase( "default-" + name ) ] =
357+
elem[ propName ] = false;
358+
} else {
359+
elem[ propName ] = false;
360+
}
349361

350362
// See #9699 for explanation of this approach (setting first, then removal)
351-
// Do not do this for boolean attributes (see #10870)
352-
if ( !isBool ) {
363+
} else {
353364
jQuery.attr( elem, name, "" );
354365
}
355-
elem.removeAttribute( getSetAttribute ? name : propName );
356366

357-
// Set corresponding property to false for boolean attributes
358-
if ( isBool && propName in elem ) {
359-
elem[ propName ] = false;
360-
}
367+
elem.removeAttribute( getSetAttribute ? name : propName );
361368
}
362369
}
363370
},
@@ -449,36 +456,48 @@ jQuery.extend({
449456
// Hook for boolean attributes
450457
boolHook = {
451458
get: function( elem, name ) {
452-
// Align boolean attributes with corresponding properties
453-
// Fall back to attribute presence where some booleans are not supported
454-
var attrNode,
455-
property = jQuery.prop( elem, name );
456-
return property === true || typeof property !== "boolean" && ( attrNode = elem.getAttributeNode(name) ) && attrNode.nodeValue !== false ?
459+
var
460+
// Use .prop to determine if this attribute is understood as boolean
461+
prop = jQuery.prop( elem, name ),
462+
463+
// Fetch it accordingly
464+
attr = typeof prop === "boolean" && elem.getAttribute( name ),
465+
detail = typeof prop === "boolean" ?
466+
467+
getSetInput && getSetAttribute ?
468+
attr != null :
469+
// oldIE fabricates an empty string for missing boolean attributes
470+
// and conflates checked/selected into attroperties
471+
ruseDefault.test( name ) ?
472+
elem[ jQuery.camelCase( "default-" + name ) ] :
473+
!!attr :
474+
475+
// fetch an attribute node for properties not recognized as boolean
476+
elem.getAttributeNode( name );
477+
478+
return detail && detail.value !== false ?
457479
name.toLowerCase() :
458480
undefined;
459481
},
460482
set: function( elem, value, name ) {
461-
var propName;
462483
if ( value === false ) {
463484
// Remove boolean attributes when set to false
464485
jQuery.removeAttr( elem, name );
465-
} else {
466-
// value is true since we know at this point it's type boolean and not false
467-
// Set boolean attributes to the same name and set the DOM property
468-
propName = jQuery.propFix[ name ] || name;
469-
if ( propName in elem ) {
470-
// Only set the IDL specifically if it already exists on the element
471-
elem[ propName ] = true;
472-
}
486+
} else if ( getSetInput && getSetAttribute || !ruseDefault.test( name ) ) {
487+
// IE<8 needs the *property* name
488+
elem.setAttribute( !getSetAttribute && jQuery.propFix[ name ] || name, name );
473489

474-
elem.setAttribute( name, name.toLowerCase() );
490+
// Use defaultChecked and defaultSelected for oldIE
491+
} else {
492+
elem[ jQuery.camelCase( "default-" + name ) ] = true;
475493
}
494+
476495
return name;
477496
}
478497
};
479498

480499
// fix oldIE value attroperty
481-
if ( !getSetAttribute || !jQuery.support.valueAttribute ) {
500+
if ( !getSetInput || !getSetAttribute ) {
482501
jQuery.attrHooks.value = {
483502
get: function( elem, name ) {
484503
var ret = elem.getAttributeNode( name );
@@ -524,12 +543,10 @@ if ( !getSetAttribute ) {
524543

525544
ret.value = value += "";
526545

527-
// Break association with cloned elements (#9646)
528-
if ( name !== "value" && value !== elem.getAttribute( name ) ) {
529-
elem.setAttribute( name, value );
530-
}
531-
532-
return value;
546+
// Break association with cloned elements by also using setAttribute (#9646)
547+
return name === "value" || value === elem.getAttribute( name ) ?
548+
value :
549+
undefined;
533550
}
534551
};
535552

‎src/support.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ jQuery.support = (function() {
122122
// Check if we can trust getAttribute("value")
123123
input = document.createElement("input");
124124
input.setAttribute( "value", "" );
125-
support.valueAttribute = input.getAttribute( "value" ) === "";
125+
support.input = input.getAttribute( "value" ) === "";
126126

127127
// Check if an input maintains its value after becoming a radio
128128
input.value = "t";

‎test/unit/attributes.js

Lines changed: 50 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ test( "attr(String)", function() {
123123
optgroup.appendChild( option );
124124
select.appendChild( optgroup );
125125

126-
equal( jQuery( option ).attr("selected"), "selected", "Make sure that a single option is selected, even when in an optgroup." );
126+
equal( jQuery( option ).prop("selected"), true, "Make sure that a single option is selected, even when in an optgroup." );
127127

128128
var $img = jQuery("<img style='display:none' width='215' height='53' src='data/1x1.jpg'/>").appendTo("body");
129129
equal( $img.attr("width"), "215", "Retrieve width attribute an an element with display:none." );
@@ -247,13 +247,14 @@ test( "attr(Hash)", function() {
247247
});
248248

249249
test( "attr(String, Object)", function() {
250-
expect( 79 );
250+
expect( 67 );
251251

252252
var div = jQuery("div").attr("foo", "bar"),
253+
i = 0,
253254
fail = false;
254255

255-
for ( var i = 0; i < div.size(); i++ ) {
256-
if ( div.get( i ).getAttribute("foo") != "bar" ) {
256+
for ( ; i < div.length; i++ ) {
257+
if ( div[ i ].getAttribute("foo") !== "bar" ) {
257258
fail = i;
258259
break;
259260
}
@@ -272,73 +273,63 @@ test( "attr(String, Object)", function() {
272273
equal( jQuery("#name").attr("name"), "something", "Set name attribute" );
273274
jQuery("#name").attr( "name", null );
274275
equal( jQuery("#name").attr("name"), undefined, "Remove name attribute" );
276+
275277
var $input = jQuery( "<input>", {
276278
name: "something",
277279
id: "specified"
278280
});
279281
equal( $input.attr("name"), "something", "Check element creation gets/sets the name attribute." );
280282
equal( $input.attr("id"), "specified", "Check element creation gets/sets the id attribute." );
281283

282-
jQuery("#check2").prop( "checked", true ).prop( "checked", false ).attr( "checked", true );
283-
equal( document.getElementById("check2").checked, true, "Set checked attribute" );
284-
equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" );
285-
equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" );
286-
jQuery("#check2").attr( "checked", false );
287-
equal( document.getElementById("check2").checked, false, "Set checked attribute" );
288-
equal( jQuery("#check2").prop("checked"), false, "Set checked attribute" );
289-
equal( jQuery("#check2").attr("checked"), undefined, "Set checked attribute" );
290-
jQuery("#text1").attr( "readonly", true );
291-
equal( document.getElementById("text1").readOnly, true, "Set readonly attribute" );
292-
equal( jQuery("#text1").prop("readOnly"), true, "Set readonly attribute" );
293-
equal( jQuery("#text1").attr("readonly"), "readonly", "Set readonly attribute" );
294-
jQuery("#text1").attr( "readonly", false );
295-
equal( document.getElementById("text1").readOnly, false, "Set readonly attribute" );
296-
equal( jQuery("#text1").prop("readOnly"), false, "Set readonly attribute" );
297-
equal( jQuery("#text1").attr("readonly"), undefined, "Set readonly attribute" );
298-
299-
jQuery("#check2").prop( "checked", true );
300-
equal( document.getElementById("check2").checked, true, "Set checked attribute" );
301-
equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" );
302-
equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" );
303-
jQuery("#check2").prop( "checked", false );
304-
equal( document.getElementById("check2").checked, false, "Set checked attribute" );
305-
equal( jQuery("#check2").prop("checked"), false, "Set checked attribute" );
306-
equal( jQuery("#check2").attr("checked"), undefined, "Set checked attribute" );
307-
308-
jQuery("#check2").attr("checked", "checked");
309-
equal( document.getElementById("check2").checked, true, "Set checked attribute with 'checked'" );
310-
equal( jQuery("#check2").prop("checked"), true, "Set checked attribute" );
311-
equal( jQuery("#check2").attr("checked"), "checked", "Set checked attribute" );
312-
313-
QUnit.reset();
284+
// As of fixing #11115, we make no promises about the effect of .attr on boolean properties
285+
$input = jQuery("#check2");
286+
$input.prop( "checked", true ).prop( "checked", false ).attr( "checked", true );
287+
equal( $input.attr("checked"), "checked", "Set checked (verified by .attr)" );
288+
$input.prop( "checked", false ).prop( "checked", true ).attr( "checked", false );
289+
equal( $input.attr("checked"), undefined, "Remove checked (verified by .attr)" );
290+
291+
$input = jQuery("#text1").prop( "readOnly", true ).prop( "readOnly", false ).attr( "readonly", true );
292+
equal( $input.attr("readonly"), "readonly", "Set readonly (verified by .attr)" );
293+
$input.prop( "readOnly", false ).prop( "readOnly", true ).attr( "readonly", false );
294+
equal( $input.attr("readonly"), undefined, "Remove readonly (verified by .attr)" );
295+
296+
$input = jQuery("#check2").attr( "checked", true ).attr( "checked", false ).prop( "checked", true );
297+
equal( $input[0].checked, true, "Set checked property (verified by native property)" );
298+
equal( $input.prop("checked"), true, "Set checked property (verified by .prop)" );
299+
equal( $input.attr("checked"), undefined, "Setting checked property doesn't affect checked attribute" );
300+
$input.attr( "checked", false ).attr( "checked", true ).prop( "checked", false );
301+
equal( $input[0].checked, false, "Clear checked property (verified by native property)" );
302+
equal( $input.prop("checked"), false, "Clear checked property (verified by .prop)" );
303+
equal( $input.attr("checked"), "checked", "Clearing checked property doesn't affect checked attribute" );
304+
305+
$input = jQuery("#check2").attr( "checked", false ).attr( "checked", "checked" );
306+
equal( $input.attr("checked"), "checked", "Set checked to 'checked' (verified by .attr)" );
314307

315308
var $radios = jQuery("#checkedtest").find("input[type='radio']");
316309
$radios.eq( 1 ).click();
317310
equal( $radios.eq( 1 ).prop("checked"), true, "Second radio was checked when clicked" );
318-
equal( $radios.attr("checked"), $radios[ 0 ].checked ? "checked" : undefined, "Known booleans do not fall back to attribute presence (#10278)" );
319-
320-
jQuery("#text1").prop( "readOnly", true );
321-
equal( document.getElementById("text1").readOnly, true, "Set readonly attribute" );
322-
equal( jQuery("#text1").prop("readOnly"), true, "Set readonly attribute" );
323-
equal( jQuery("#text1").attr("readonly"), "readonly", "Set readonly attribute" );
324-
jQuery("#text1").prop( "readOnly", false );
325-
equal( document.getElementById("text1").readOnly, false, "Set readonly attribute" );
326-
equal( jQuery("#text1").prop("readOnly"), false, "Set readonly attribute" );
327-
equal( jQuery("#text1").attr("readonly"), undefined, "Set readonly attribute" );
328-
329-
jQuery("#name").attr( "maxlength", "5" );
330-
equal( document.getElementById("name").maxLength, 5, "Set maxlength attribute" );
331-
jQuery("#name").attr( "maxLength", "10" );
332-
equal( document.getElementById("name").maxLength, 10, "Set maxlength attribute" );
311+
equal( $radios.eq( 0 ).attr("checked"), "checked", "First radio is still [checked]" );
312+
313+
$input = jQuery("#text1").attr( "readonly", false ).prop( "readOnly", true );
314+
equal( $input[0].readOnly, true, "Set readonly property (verified by native property)" );
315+
equal( $input.prop("readOnly"), true, "Set readonly property (verified by .prop)" );
316+
$input.attr( "readonly", true ).prop( "readOnly", false );
317+
equal( $input[0].readOnly, false, "Clear readonly property (verified by native property)" );
318+
equal( $input.prop("readOnly"), false, "Clear readonly property (verified by .prop)" );
319+
320+
$input = jQuery("#name").attr( "maxlength", "5" );
321+
equal( $input[0].maxLength, 5, "Set maxlength (verified by native property)" );
322+
$input.attr( "maxLength", "10" );
323+
equal( $input[0].maxLength, 10, "Set maxlength (verified by native property)" );
333324

334325
// HTML5 boolean attributes
335326
var $text = jQuery("#text1").attr({
336327
"autofocus": true,
337328
"required": true
338329
});
339-
equal( $text.attr("autofocus"), "autofocus", "Set boolean attributes to the same name" );
340-
equal( $text.attr( "autofocus", false ).attr("autofocus"), undefined, "Setting autofocus attribute to false removes it" );
341-
equal( $text.attr("required"), "required", "Set boolean attributes to the same name" );
330+
equal( $text.attr("autofocus"), "autofocus", "Reading autofocus attribute yields 'autofocus'" );
331+
equal( $text.attr( "autofocus", false ).attr("autofocus"), undefined, "Setting autofocus to false removes it" );
332+
equal( $text.attr("required"), "required", "Reading required attribute yields 'required'" );
342333
equal( $text.attr( "required", false ).attr("required"), undefined, "Setting required attribute to false removes it" );
343334

344335
var $details = jQuery("<details open></details>").appendTo("#qunit-fixture");
@@ -368,13 +359,14 @@ test( "attr(String, Object)", function() {
368359
strictEqual( $elem.attr("nonexisting"), undefined, "attr(name, value) works correctly on comment and text nodes (bug #7500)." );
369360
});
370361

362+
// Register the property name to avoid generating a new global when testing window
363+
Globals.register("nonexisting");
371364
jQuery.each( [ window, document, obj, "#firstp" ], function( i, elem ) {
372-
// use iframeCallback to avoid generating a new global when testing window
373-
var oldVal = elem.iframeCallback,
365+
var oldVal = elem.nonexisting,
374366
$elem = jQuery( elem );
375367
strictEqual( $elem.attr("nonexisting"), undefined, "attr works correctly for non existing attributes (bug #7500)." );
376-
equal( $elem.attr( "iframeCallback", "foo" ).attr("iframeCallback"), "foo", "attr falls back to prop on unsupported arguments" );
377-
elem.iframeCallback = oldVal;
368+
equal( $elem.attr( "nonexisting", "foo" ).attr("nonexisting"), "foo", "attr falls back to prop on unsupported arguments" );
369+
elem.nonexisting = oldVal;
378370
});
379371

380372
var table = jQuery("#table").append("<tr><td>cell</td></tr><tr><td>cell</td><td>cell</td></tr><tr><td>cell</td><td>cell</td></tr>"),

‎test/unit/selector.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ test("class - jQuery only", function() {
3131
});
3232

3333
test("attributes - jQuery only", function() {
34-
expect( 4 );
34+
expect( 6 );
3535

3636
t( "Find elements with a tabindex attribute", "[tabindex]", ["listWithTabIndex", "foodWithNegativeTabIndex", "linkWithTabIndex", "linkWithNegativeTabIndex", "linkWithNoHrefWithTabIndex", "linkWithNoHrefWithNegativeTabIndex"] );
3737

@@ -53,6 +53,14 @@ test("attributes - jQuery only", function() {
5353
ok( jQuery("<input type='text' value='12600'/>").prop( "value", "option" ).is(":input[value='12600']"),
5454
":input[value=foo] selects text input by attribute"
5555
);
56+
57+
// #11115
58+
ok( jQuery("<input type='checkbox' checked='checked'/>").prop( "checked", false ).is("[checked]"),
59+
"[checked] selects by attribute (positive)"
60+
);
61+
ok( !jQuery("<input type='checkbox'/>").prop( "checked", true ).is("[checked]"),
62+
"[checked] selects by attribute (negative)"
63+
);
5664
});
5765

5866
if ( jQuery.css ) {
@@ -83,7 +91,7 @@ test("disconnected nodes", function() {
8391
var $opt = jQuery("<option></option>").attr("value", "whipit").appendTo("#qunit-fixture").detach();
8492
equal( $opt.val(), "whipit", "option value" );
8593
equal( $opt.is(":selected"), false, "unselected option" );
86-
$opt.attr("selected", true);
94+
$opt.prop("selected", true);
8795
equal( $opt.is(":selected"), true, "selected option" );
8896

8997
var $div = jQuery("<div/>");

‎test/unit/traversing.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,11 @@ test("not(Selector|undefined)", function() {
329329
equal( jQuery("#qunit-fixture > p#ap > a").not("#google").length, 2, "not('selector')" );
330330
deepEqual( jQuery("p").not(".result").get(), q("firstp", "ap", "sndp", "en", "sap", "first"), "not('.class')" );
331331
deepEqual( jQuery("p").not("#ap, #sndp, .result").get(), q("firstp", "en", "sap", "first"), "not('selector, selector')" );
332-
deepEqual( jQuery("#form option").not("option.emptyopt:contains('Nothing'),[selected],[value='1']").get(), q("option1c", "option1d", "option2c", "option3d", "option3e", "option4e","option5b"), "not('complex selector')");
332+
deepEqual(
333+
jQuery("#form option").not("option.emptyopt:contains('Nothing'),optgroup *,[value='1']").get(),
334+
q("option1c", "option1d", "option2c", "option2d", "option3c", "option3d", "option3e", "option4d", "option4e", "option5a", "option5b"),
335+
"not('complex selector')"
336+
);
333337

334338
deepEqual( jQuery("#ap *").not("code").get(), q("google", "groups", "anchor1", "mark"), "not('tag selector')" );
335339
deepEqual( jQuery("#ap *").not("code, #mark").get(), q("google", "groups", "anchor1"), "not('tag, ID selector')" );

0 commit comments

Comments
 (0)