Attributes: Support the `until-found` value for the `hidden` attribute #5607

mgol · 2025-01-05T12:47:03Z

Summary

The hidden attribute used to be a boolean one but it gained a new until-found eventually. This led us to change the way we handle boolean attributes in jQuery 4.0 in gh-5452 to avoid these issues in the future.

That said, currently from the attributes we treat as boolean only hidden has gained an extra value, let's support it.

Ref gh-5388
Ref gh-5452

The cost is currently quite high:

3.x-stable @fc874a0e125b01c2d91b4a8a06213bf1f1e5bfb6
   raw     gz     br Filename
  +157    +66    +58 dist/jquery.min.js
  +157    +52    +36 dist/jquery.slim.min.js

It's possible it could be lowered if we removed the infra for adding future attributes to the list and just mentioned hidden & until-found explicitly in both the getter & setter.

Another alternative is to avoid changing the 3.x line and instead add a note to the docs, showing how to define an attrHook for hidden that will achieve the same result.

Checklist

New tests have been added to show the fix or feature works
If needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com

mgol · 2025-01-05T12:54:57Z

Mentioning hidden & until-found directly results in the following size diff:

3.x-stable @fc874a0e125b01c2d91b4a8a06213bf1f1e5bfb6
   raw     gz     br Filename
  +139    +51     +5 dist/jquery.min.js
  +139    +43    +34 dist/jquery.slim.min.js

timmywil

LGTM!

mgol · 2025-01-13T17:52:55Z

@gibson042 I pushed the smaller version in the second commit, you can compare the two & suggest improvements.

gibson042

Compression pass:

   raw     gz Compared to last run                                             
   -52     -6 dist/jquery.js                                                   
   -13    -17 dist/jquery.min.js

gibson042 · 2025-01-13T18:41:36Z

src/attributes/attr.js

+		} else if (
+			name.toLowerCase() !== "hidden" ||
+				String( value ).toLowerCase() !== "until-found"
+		) {
 			elem.setAttribute( name, name );
+		} else {
+			elem.setAttribute( name, value );


Suggested change

} else if (

name.toLowerCase() !== "hidden" ||

String( value ).toLowerCase() !== "until-found"

) {

elem.setAttribute( name, name );

} else {

elem.setAttribute( name, value );

} else {

elem.setAttribute( name, name.toLowerCase() === "hidden" &&

strValue.toLowerCase() === "until-found" ? strValue : name );

(with var strValue = String( value );)

gibson042 · 2025-01-13T18:42:31Z

src/attributes/attr.js

+			if ( ret != null ) {
+				if ( lowercaseName !== "hidden" ||
+					ret.toLowerCase() !== "until-found"
+				) {
+					ret = lowercaseName;
+				}
+			}


Suggested change

if ( ret != null ) {

if ( lowercaseName !== "hidden" ||

ret.toLowerCase() !== "until-found"

) {

ret = lowercaseName;

}

}

ret = ret == null || lowercaseName === "hidden" &&

ret.toLowerCase() === "until-found" ? ret : lowercaseName;

mgol · 2025-01-13T23:15:46Z

@gibson042 I applied your suggestions but I got a different diff compared to my second commit:

3.x-hidden-until-found @1f668bf0a2f354a35bd0e9b125fb398901b63c90
   raw     gz     br Filename
   -13     -9    +67 dist/jquery.min.js
   -13     -6    -21 dist/jquery.slim.min.js

I pushed it, can you have a look?

The diff from 3.x-stable right now:

3.x-stable @fc874a0e125b01c2d91b4a8a06213bf1f1e5bfb6
   raw     gz     br Filename
  +126    +42    +72 dist/jquery.min.js
  +126    +37    +13 dist/jquery.slim.min.js

gibson042 · 2025-01-14T18:52:29Z

I applied your suggestions but I got a different diff

@mgol Confirmed your numbers; I must have been doing something slightly different before. I think we're close to a local maximum, but vs. the current state,

   raw     gz     br Filename
    +2     -4    -39 dist/jquery.min.js
    +2     -1     +7 dist/jquery.slim.min.js

is possible by refactoring variable declarations:

boolHook = {
    set: function( elem, value, name ) {
        var strValue = String( value ),
            lowercaseName = name.toLowerCase();

        if ( value === false ) {

            // Remove boolean attributes when set to false
            jQuery.removeAttr( elem, name );
        } else {
            elem.setAttribute( name,
                lowercaseName === "hidden" &&
                        strValue.toLowerCase() === "until-found" ?
                    strValue :
                    name
            );
        }
        return name;
    }
};

jQuery.each( jQuery.expr.match.bool.source.match( /\w+/g ), function( _i, name ) {
    var getter = attrHandle[ name ] || jQuery.find.attr;

    attrHandle[ name ] = function( elem, name, isXML ) {
        var ret,
            lowercaseName = name.toLowerCase(),
            handle = attrHandle[ lowercaseName ];

        if ( !isXML ) {

            // Avoid an infinite loop by temporarily removing this function from the getter
            attrHandle[ lowercaseName ] = ret;

            ret = getter( elem, name, isXML );
            ret = ret == null ||
                    ( lowercaseName === "hidden" && ret.toLowerCase() === "until-found" ) ?
                ret :
                lowercaseName;

            attrHandle[ lowercaseName ] = handle;
        }
        return ret;
    };
} );

The `hidden` attribute used to be a boolean one but it gained a new `until-found` eventually. This led us to change the way we handle boolean attributes in jQuery 4.0 in jquerygh-5452 to avoid these issues in the future. That said, currently from the attributes we treat as boolean only `hidden` has gained an extra value, let's support it. Ref jquerygh-5388 Ref jquerygh-5452

…ttribute

…dden` attribute

mgol · 2025-01-16T13:11:37Z

@gibson042 I am getting a different diff for this version of boolHook vs. what's in the PR:

3.x-hidden-until-found @9c8e3e3c41927744995729d9270b051cdc5cde5e
   raw     gz     br Filename
    +4     +2    -43 dist/jquery.min.js
    +4     +1    +10 dist/jquery.slim.min.js

mgol · 2025-01-16T13:13:20Z

I'm going to merge it as-is; feel free to submit a PR with various size improvements if you find them.

The `hidden` attribute used to be a boolean one but it gained a new `until-found` eventually. This led us to change the way we handle boolean attributes in jQuery 4.0 in jquerygh-5452 to avoid these issues in the future. We haven't added an explicit test for the `"until-found"` value of the `hidden` attribute which triggered this decision so far, though. Backport the test from jquerygh-5607 which landed on `3.x-stable` so that we do test it. Ref jquerygh-5452 Ref jquerygh-5607 (cherry picked from commit 85290c5)

The `hidden` attribute used to be a boolean one but it gained a new `until-found` eventually. This led us to change the way we handle boolean attributes in jQuery 4.0 in gh-5452 to avoid these issues in the future. We haven't added an explicit test for the `"until-found"` value of the `hidden` attribute which triggered this decision so far, though. Backport the test from gh-5607 which landed on `3.x-stable` so that we do test it. Closes gh-5619 Ref gh-5452 Ref gh-5607 (cherry picked from commit 85290c5)

mgol added the Attributes label Jan 5, 2025

mgol self-assigned this Jan 5, 2025

mgol force-pushed the 3.x-hidden-until-found branch 2 times, most recently from 6ee4613 to 2aba711 Compare January 5, 2025 12:52

mgol marked this pull request as ready for review January 5, 2025 12:55

mgol added Needs review Discuss in Meeting labels Jan 5, 2025

timmywil approved these changes Jan 13, 2025

View reviewed changes

timmywil removed the Discuss in Meeting label Jan 13, 2025

gibson042 approved these changes Jan 13, 2025

View reviewed changes

mgol added this to the 3.7.2 milestone Jan 13, 2025

mgol added the Discuss in Meeting label Jan 14, 2025

mgol force-pushed the 3.x-hidden-until-found branch from 32391cf to e47c8b5 Compare January 14, 2025 16:45

mgol added 3 commits January 16, 2025 14:05

fixup! Attributes: Support the until-found value for the hidden a…

6ac7a97

…ttribute

fixup! fixup! Attributes: Support the until-found value for the `hi…

9c8e3e3

…dden` attribute

mgol force-pushed the 3.x-hidden-until-found branch from e47c8b5 to 9c8e3e3 Compare January 16, 2025 13:11

mgol merged commit 85290c5 into jquery:3.x-stable Jan 16, 2025
17 checks passed

mgol deleted the 3.x-hidden-until-found branch January 16, 2025 13:15

mgol removed Discuss in Meeting Needs review labels Jan 16, 2025

mgol mentioned this pull request Jan 27, 2025

Tests: Backport the hidden="until-found" attr tests from 3.x-stable #5619

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attributes: Support the `until-found` value for the `hidden` attribute #5607

Attributes: Support the `until-found` value for the `hidden` attribute #5607

mgol commented Jan 5, 2025 •

edited

Loading

mgol commented Jan 5, 2025

timmywil left a comment

mgol commented Jan 13, 2025

gibson042 left a comment

gibson042 Jan 13, 2025

gibson042 Jan 13, 2025

mgol commented Jan 13, 2025

gibson042 commented Jan 14, 2025

mgol commented Jan 16, 2025

mgol commented Jan 16, 2025

Attributes: Support the until-found value for the hidden attribute #5607

Attributes: Support the until-found value for the hidden attribute #5607

Conversation

mgol commented Jan 5, 2025 • edited Loading

Summary

Checklist

mgol commented Jan 5, 2025

timmywil left a comment

Choose a reason for hiding this comment

mgol commented Jan 13, 2025

gibson042 left a comment

Choose a reason for hiding this comment

gibson042 Jan 13, 2025

Choose a reason for hiding this comment

gibson042 Jan 13, 2025

Choose a reason for hiding this comment

mgol commented Jan 13, 2025

gibson042 commented Jan 14, 2025

mgol commented Jan 16, 2025

mgol commented Jan 16, 2025

Attributes: Support the `until-found` value for the `hidden` attribute #5607

Attributes: Support the `until-found` value for the `hidden` attribute #5607

mgol commented Jan 5, 2025 •

edited

Loading