🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@andersk
Copy link
Contributor

@andersk andersk commented Sep 3, 2022

Summary

Fixes gh-5105

Checklist

  • New tests have been added to show the fix or feature works
  • Grunt build and unit tests pass locally with these changes
  • If needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 3, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: andersk / name: Anders Kaseorg (df326d9)

@andersk andersk force-pushed the undefined-custom-property branch from 1d8ec8d to df326d9 Compare September 3, 2022 03:24
@mgol mgol added CSS Needs review Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Sep 3, 2022
@mgol mgol added this to the 3.6.2 milestone Sep 3, 2022
@mgol
Copy link
Member

mgol commented Sep 3, 2022

Thanks for the PR with a test!

We’ll discuss what approach we want to take here and we’ll get back to you.

@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Sep 12, 2022
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment, otherwise it looks good to me. Thanks!

@andersk andersk force-pushed the undefined-custom-property branch from df326d9 to 92540d3 Compare September 12, 2022 20:29
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the tests failed in Chrome; can you have a look?

Disregard the selector :has failures, they are caused by #5098 and will be fixed when #5107 lands.

@andersk andersk force-pushed the undefined-custom-property branch from 92540d3 to 4fb461c Compare September 14, 2022 05:06
@andersk
Copy link
Contributor Author

andersk commented Sep 14, 2022

This is a cross-browser difference with --empty: ;. In Firefox, .getPropertyValue("--empty") returns the empty string "", while in Chrome, it returns a space " ".

I’ve changed the test to --empty:; without the space to avoid this issue. We can consider further normalizing this cross-browser difference, but since that’s neither a crash nor a regression, I think we should consider it separately.

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks! I'll need to ask you to rebase the PR once #5107 lands so that the build is green here. Other than that, LGTM.

Fixes jquerygh-5105

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk force-pushed the undefined-custom-property branch from 4fb461c to ee6718a Compare September 19, 2022 19:03
@andersk
Copy link
Contributor Author

andersk commented Sep 19, 2022

Rebased.

@mgol mgol merged commit ed306c0 into jquery:main Sep 19, 2022
@andersk andersk deleted the undefined-custom-property branch September 19, 2022 21:11
andersk added a commit to andersk/jquery that referenced this pull request Sep 19, 2022
Fixes jquerygh-5105
Closes jquerygh-5106

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
(cherry picked from commit ed306c0)
@andersk
Copy link
Contributor Author

andersk commented Sep 19, 2022

mgol pushed a commit that referenced this pull request 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

Landed on main in ed306c0 and on 3.x-stable in c0db6d7. Thanks!

@mgol mgol removed the Needs review label Sep 19, 2022
mgol added a commit to mgol/jquery that referenced this pull request Sep 21, 2022
The spec requires that CSS variable values are trimmed. In browsers that do this
- mainly, Safari, but also Firefox if the value only has leading whitespace
- we currently return `undefined`; in other browsers, we return an empty string
as the logic to fall back to `undefined` happens before trimming.

This commit adds another explicit callback to `undefined`.

Also, more explicit comments about behaviors we need to work around in various
browsers have been added.

Ref jquerygh-5106
@mgol
Copy link
Member

mgol commented Sep 21, 2022

I’ve changed the test to --empty:; without the space to avoid this issue. We can consider further normalizing this cross-browser difference, but since that’s neither a crash nor a regression, I think we should consider it separately.

I submitted #5120 to handle this case.

mgol added a commit to mgol/jquery that referenced this pull request Sep 21, 2022
The spec requires that CSS variable values are trimmed. In browsers that do this
- mainly, Safari, but also Firefox if the value only has leading whitespace
- we currently return `undefined`; in other browsers, we return an empty string
as the logic to fall back to `undefined` happens before trimming.

This commit adds another explicit callback to `undefined` to have it consistent
across browsers.

Also, more explicit comments about behaviors we need to work around in various
browsers have been added.

Ref jquerygh-5106
mgol added a commit that referenced this pull request Oct 3, 2022
The spec requires that CSS variable values are trimmed. In browsers that do
this - mainly, Safari, but also Firefox if the value only has leading
whitespace - we currently return undefined; in other browsers, we return
an empty string as the logic to fall back to undefined happens before
trimming.

This commit adds another explicit callback to `undefined` to have it consistent
across browsers.

Also, more explicit comments about behaviors we need to work around in various
browsers have been added.

Closes gh-5120
Ref gh-5106
mgol added a commit that referenced this pull request Oct 3, 2022
The spec requires that CSS variable values are trimmed. In browsers that do
this - mainly, Safari, but also Firefox if the value only has leading
whitespace - we currently return undefined; in other browsers, we return
an empty string as the logic to fall back to undefined happens before
trimming.

This commit adds another explicit callback to `undefined` to have it consistent
across browsers.

Also, more explicit comments about behaviors we need to work around in various
browsers have been added.

Closes gh-5120
Ref gh-5106

(cherry picked from commit 7eb0019)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

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

3 participants