8000 Attributes: Make `.attr( name, false )` remove for all non-ARIA attrs by mgol · Pull Request #5452 · jquery/jquery · GitHub
[go: up one dir, main page]
Skip to content

Attributes: Make .attr( name, false ) remove for all non-ARIA attrs #5452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Attributes: Make .attr( name, false ) remove for all non-ARIA attrs #5452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 8 additions & 30 deletions src/attributes/attr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ jQuery.extend( {
}

if ( value !== undefined ) {
if ( value === null ) {
if ( value === null ||

// For compat with previous handling of boolean attributes,
// remove when `false` passed. For ARIA attributes -
// many of which recognize a `"false"` value - continue to
// set the `"false"` value as jQuery <4 did.
( value === false && name.toLowerCase().indexOf( "aria-" ) !== 0 ) ) {

jQuery.removeAttr( elem, name );
return;
}
Expand Down Expand Up @@ -96,32 +103,3 @@ if ( isIE ) {
}
};
}

// HTML boolean attributes have special behavior:
// we consider the lowercase name to be the only valid value, so
// getting (if the attribute is present) normalizes to that, as does
// setting to any non-`false` value (and setting to `false` removes the attribute).
// See https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes
jQuery.each( (
"checked selected async autofocus autoplay controls defer disabled " +
"hidden ismap loop multiple open readonly required scoped"
).split( " " ), function( _i, name ) {
jQuery.attrHooks[ name ] = {
get: function( elem ) {
return elem.getAttribute( name ) != null ?
name.toLowerCase() :
null;
},

set: function( elem, value, name ) {
if ( value === false ) {

// Remove boolean attributes when set to false
jQuery.removeAttr( elem, name );
} else {
elem.setAttribute( name, name );
}
return name;
}
};
} );
86 changes: 72 additions & 14 deletions test/unit/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,12 @@ QUnit.test( "attr(String, Object)", function( assert ) {
assert.equal( $input[ 0 ].selected, true, "Setting selected updates property (verified by native property)" );

$input = jQuery( "#check2" );
$input.prop( "checked", true ).prop( "checked", false ).attr( "checked", true );
$input.prop( "checked", true ).prop( "checked", false ).attr( "checked", "checked" );
assert.equal( $input.attr( "checked" ), "checked", "Set checked (verified by .attr)" );
$input.prop( "checked", false ).prop( "checked", true ).attr( "checked", false );
assert.equal( $input.attr( "checked" ), undefined, "Remove checked (verified by .attr)" );

$input = jQuery( "#text1" ).prop( "readOnly", true ).prop( "readOnly", false ).attr( "readonly", true );
$input = jQuery( "#text1" ).prop( "readOnly", true ).prop( "readOnly", false ).attr( "readonly", "readonly" );
assert.equal( $input.attr( "readonly" ), "readonly", "Set readonly (verified by .attr)" );
$input.prop( "readOnly", false ).prop( "readOnly", true ).attr( "readonly", false );
assert.equal( $input.attr( "readonly" ), undefined, "Remove readonly (verified by .attr)" );
Expand All @@ -318,7 +318,7 @@ QUnit.test( "attr(String, Object)", function( assert ) {
assert.equal( $input[ 0 ].checked, true, "Set checked property (verified by native property)" );
assert.equal( $input.prop( "checked" ), true, "Set checked property (verified by .prop)" );
assert.equal( $input.attr( "checked" ), undefined, "Setting checked property doesn't affect checked attribute" );
$input.attr( "checked", false ).attr( "checked", true ).prop( "checked", false );
$input.attr( "checked", false ).attr( "checked", "checked" ).prop( "checked", false );
assert.equal( $input[ 0 ].checked, false, "Clear checked property (verified by native property)" );
assert.equal( $input.prop( "checked" ), false, "Clear checked property (verified by .prop)" );
assert.equal( $input.attr( "checked" ), "checked", "Clearing checked property doesn't affect checked attribute" );
Expand All @@ -345,22 +345,22 @@ QUnit.test( "attr(String, Object)", function( assert ) {

// HTML5 boolean attributes
$text = jQuery( "#text1" ).attr( {
"autofocus": true,
"required": true
"autofocus": "autofocus",
"required": "required"
} );
assert.equal( $text.attr( "autofocus" ), "autofocus", "Reading autofocus attribute yields 'autofocus'" );
assert.equal( $text.attr( "autofocus", false ).attr( "autofocus" ), undefined, "Setting autofocus to false removes it" );
assert.equal( $text.attr( "required" ), "required", "Reading required attribute yields 'required'" );
assert.equal( $text.attr( "required", false ).attr( "required" ), undefined, "Setting required attribute to false removes it" );

$details = jQuery( "<details open></details>" ).appendTo( "#qunit-fixture" );
assert.equal( $details.attr( "open" ), "open", "open attribute presence indicates true" );
assert.equal( $details.attr( "open" ), "", "open attribute presence indicates true" );
assert.equal( $details.attr( "open", false ).attr( "open" ), undefined, "Setting open attribute to false removes it" );

$text.attr( "data-something", true );
assert.equal( $text.attr( "data-something" ), "true", "Set data attributes" );
assert.equal( $text.data( "something" ), true, "Setting data attributes are not affected by boolean settings" );
$text.attr( "data-another", false );
$text.attr( "data-another", "false" );
assert.equal( $text.attr( "data-another" ), "false", "Set data attributes" );
assert.equal( $text.data( "another" ), false, "Setting data attributes are not affected by boolean settings" );
assert.equal( $text.attr( "aria-disabled", false ).attr( "aria-disabled" ), "false", "Setting aria attributes are not affected by boolean settings" );
Expand Down Expand Up @@ -1790,21 +1790,79 @@ QUnit.test( "non-lowercase boolean attribute getters should not crash", function

var elem = jQuery( "<input checked required autofocus type='checkbox'>" );

jQuery.each( {
checked: "Checked",
required: "requiRed",
autofocus: "AUTOFOCUS"
}, function( lowercased, original ) {
[
"Checked", "requiRed", "AUTOFOCUS"
].forEach( function( inconsistentlyCased ) {
try {
assert.strictEqual( elem.attr( original ), lowercased,
"The '" + this + "' attribute getter should return the lowercased name" );
assert.strictEqual( elem.attr( inconsistentlyCased ), "",
"The '" + this + "' attribute getter should return an empty string" );
} catch ( e ) {
assert.ok( false, "The '" + this + "' attribute getter threw" );
}
} );
} );


QUnit.test( "false setter removes non-ARIA attrs (gh-5388)", function( assert ) {
assert.expect( 24 );

var elem = jQuery( "<input" +
" checked required autofocus" +
" type='checkbox'" +
" title='Example title'" +
" class='test-class'" +
" style='color: brown'" +
" aria-hidden='true'" +
" aria-checked='true'" +
" aria-label='Example ARIA label'" +
" data-prop='Example data value'" +
" data-title='Example data title'" +
" data-true='true'" +
">" );

function testFalseSetter( attributes, options ) {
var removal = options.removal;

attributes.forEach( function( attrName ) {
assert.ok( elem.attr( attrName ) != null,
"Attribute '" + attrName + "': initial defined value" );
elem.attr( attrName, false );

if ( removal ) {
assert.strictEqual( elem.attr( attrName ), undefined,
"Attribute '" + attrName + "' removed" );
} else {
assert.strictEqual( elem.attr( attrName ), "false",
"Attribute '" + attrName + "' set to 'false'" );
}
} );
}

// Boolean attributes
testFalseSetter(
[ "checked", "required", "autofocus" ],
{ removal: true }
);

// Regular attributes
testFalseSetter(
[ "title", "class", "style" ],
{ removal: true }
);

// `aria-*` attributes
testFalseSetter(
[ "aria-hidden", "aria-checked", "aria-label" ],
{ removal: false }
);

// `data-*` attributes
testFalseSetter(
[ "data-prop", "data-title", "data-true" ],
{ removal: true }
);
} );

// Test trustedTypes support in browsers where they're supported (currently Chrome 83+).
// Browsers with no TrustedScriptURL support still run tests on object wrappers with
// a proper `toString` function.
Expand Down
8 changes: 4 additions & 4 deletions test/unit/selector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,10 +948,10 @@ QUnit.test( "pseudo - nth-last-of-type", function( assert ) {
QUnit[ QUnit.jQuerySelectors ? "test" : "skip" ]( "pseudo - has", function( assert ) {
assert.expect( 4 );

assert.t( "Basic test", "p:has(a)", [ "firstp", "ap", "en", "sap" ] );
assert.t( "Basic test (irrelevant whitespace)", "p:has( a )", [ "firstp", "ap", "en", "sap" ] );
assert.t( "Nested with overlapping candidates",
"#qunit-fixture div:has(div:has(div:not([id])))",
assert.selectInFixture( "Basic test", "p:has(a)", [ "firstp", "ap", "en", "sap" ] );
assert.selectInFixture( "Basic test (irrelevant whitespace)", "p:has( a )", [ "firstp", "ap", "en", "sap" ] );
assert.selectInFixture( "Nested with overlapping candidates",
"div:has(div:has(div:not([id])))",
[ "moretests", "t2037", "fx-test-group", "fx-queue" ] );

// Support: Safari 15.4+, Chrome 105+
Expand Down
0