Skip to content
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.

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

.css("--custom") throws TypeError: ret is undefined (3.6.0 → 3.6.1 regression) #5105

Closed
andersk opened this issue Sep 3, 2022 · 4 comments · Fixed by #5106
Closed

.css("--custom") throws TypeError: ret is undefined (3.6.0 → 3.6.1 regression) #5105

andersk opened this issue Sep 3, 2022 · 4 comments · Fixed by #5106

Comments

@andersk
Copy link
Contributor

andersk commented Sep 3, 2022

Description

In jQuery 3.6.0, $element.css("--custom") correctly returned undefined for a custom property that isn’t set. But in jQuery 3.6.1, it crashes with TypeError: ret is undefined.

This breaks the Zulip settings dialog.

This was presumably introduced by #4930 (cc @fecore1 @mgol).

Link to test case

https://codepen.io/anderskaseorg/pen/rNvNZWW

andersk added a commit to andersk/zulip that referenced this issue Sep 3, 2022
jQuery is held at 3.6.0 due to
jquery/jquery#5105.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
andersk added a commit to andersk/jquery that referenced this issue Sep 3, 2022
Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/jquery that referenced this issue Sep 3, 2022
Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/zulip that referenced this issue Sep 3, 2022
jQuery is held at 3.6.0 due to
jquery/jquery#5105.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
andersk added a commit to andersk/zulip that referenced this issue Sep 3, 2022
jQuery is held at 3.6.0 due to
jquery/jquery#5105.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
@mgol mgol added Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. CSS labels Sep 3, 2022
@mgol
Copy link
Member

mgol commented Sep 3, 2022

Thanks for the report. That said, I'm not sure the undefined return value was correct either. We are using .getPropertyValue() for custom property values and it returns an empty string for unknown CSS custom properties. We just then fallback to computed[ value ] which will be undefined. The logic is at:
https://github.com/jquery/jquery/blob/3.6.1/src/css/curCSS.js#L31

That said, for regular unknown properties that is the logic we are using. I guess we should discuss what's our goal here.

@mgol mgol added this to the 3.6.2 milestone Sep 3, 2022
@andersk
Copy link
Contributor Author

andersk commented Sep 3, 2022

Yeah, jQuery has given undefined here for standard properties since 1.8.0, and for custom properties since they were first supported in 3.2.0. “Correct” or not, it seems to me that a patch release isn’t a good time to break this compatibility.

@mgol
Copy link
Member

mgol commented Sep 3, 2022

I agree with that but there’s also a case with a custom property set to an empty string or a whitespace-only string. IMO then we should return an empty string, not undefined. I’ll need to check if this is possible with the Web APIs we have.

andersk added a commit to andersk/zulip that referenced this issue Sep 5, 2022
jQuery is held at 3.6.0 due to
jquery/jquery#5105.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
timabbott pushed a commit to zulip/zulip that referenced this issue Sep 6, 2022
jQuery is held at 3.6.0 due to
jquery/jquery#5105.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
@timmywil timmywil added Has Pull Request and removed Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Sep 12, 2022
andersk added a commit to andersk/jquery that referenced this issue Sep 12, 2022
Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/jquery that referenced this issue Sep 14, 2022
Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/jquery that referenced this issue Sep 19, 2022
Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
mgol pushed a commit that referenced this issue Sep 19, 2022
Fixes gh-5105
Closes gh-5106

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
andersk added a commit to andersk/jquery that referenced this issue Sep 19, 2022
Fixes jquerygh-5105
Closes jquerygh-5106

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
(cherry picked from commit ed306c0)
mgol pushed a commit that referenced this issue Sep 19, 2022
Fixes gh-5105
Closes gh-5106

Signed-off-by: Anders Kaseorg <andersk@mit.edu>

(cherry picked from commit ed306c0)
@mgol
Copy link
Member

mgol commented Sep 19, 2022

Fix landed on main in ed306c0 and on 3.x-stable in c0db6d7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants