Skip to content

CSS: Return undefined for whitespace-only CSS variable values #5120

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

Merged
merged 1 commit into from
Oct 3, 2022

Conversation

mgol
Copy link
Member

@mgol mgol commented Sep 21, 2022

Summary

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 PR 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 gh-5106

+3 bytes

I verified that IE 11 passes all the tests with these changes.

Checklist

Sorry, something went wrong.

@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 21, 2022
@mgol mgol added this to the 3.6.2 milestone Sep 21, 2022
@mgol mgol self-assigned this 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 mgol force-pushed the css-vars-whitespace branch from 8f953da to 31eeea5 Compare September 21, 2022 12:06
Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

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

LGTM. Makes it consistent.

@mgol mgol removed Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. Needs review labels Oct 3, 2022
@mgol mgol merged commit 7eb0019 into jquery:main Oct 3, 2022
@mgol mgol deleted the css-vars-whitespace branch October 3, 2022 16:10
mgol added a commit to mgol/jquery that referenced this pull request Oct 3, 2022
…ry#5120)

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 jquerygh-5120
Ref jquerygh-5106

(cherry picked from commit 7eb0019)
@mgol
Copy link
Member Author

mgol commented Oct 3, 2022

Landed in 7eb0019 on main & in 8bea1de on 3.x-stable.

wmfgerrit pushed a commit to wikimedia/mediawiki that referenced this pull request Aug 29, 2023
* Cross-browser: Improve selector engine to accomodate recent changes
  in Safari and the cross-browser differences that introduces around `:has()`.

  Note that the built-in `querySelectorAll()` sometimes throws
  exceptions, which jQuery avoids from affecting your code.

* Cross-browser: Ensure that `$el.css('--some-variable')`
  when the value is an empty string, returns undefined. Including in
  current versions of Firefox and last years versions of Chrome, which
  behave non-standardly.
  jquery/jquery#5120

* Cross-browser: Avoid crash in Safari 16 when using `<template>`
  elements, due to change in document.adoptNode() behaviour.

* Optimise size of various methods by removing code for obsolete browser
  versions. Primarily, removal/inlining of Sizzle engine, and removal
  of old code for Firefox versions that behaved differently around
  SVG or XML elements.

  About 4KB smaller in decoded size for JS parsing.
  About 2KB smaller in transfer size.

* Optimise runtime of first `$()` call by removing obsolete support.
* Optimise manipulation methods like `append()`.
* Optimise focus events by using native events when possible/reliable.

Highlights from
https://blog.jquery.com/2022/12/13/jquery-3-6-2-released/
https://blog.jquery.com/2022/12/20/jquery-3-6-3-released-a-quick-selector-fix/
https://blog.jquery.com/2023/03/08/jquery-3-6-4-released-selector-forgiveness
https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/
https://blog.jquery.com/2023/08/28/jquery-3-7-1-released-reliable-table-row-dimensions/

Change-Id: I144069ce80127692b5c0ff91c4e55383f4013e2f
wmfgerrit pushed a commit to wikimedia/mediawiki that referenced this pull request Aug 29, 2023
* Cross-browser: Improve selector engine to accomodate recent changes
  in Safari and the cross-browser differences that introduces around `:has()`.

  Note that the built-in `querySelectorAll()` sometimes throws
  exceptions, which jQuery avoids from affecting your code.

* Cross-browser: Ensure that `$el.css('--some-variable')`
  when the value is an empty string, returns undefined. Including in
  current versions of Firefox and last years versions of Chrome, which
  behave non-standardly.
  jquery/jquery#5120

* Cross-browser: Avoid crash in Safari 16 when using `<template>`
  elements, due to change in document.adoptNode() behaviour.

* Optimise size of various methods by removing code for obsolete browser
  versions. Primarily, removal/inlining of Sizzle engine, and removal
  of old code for Firefox versions that behaved differently around
  SVG or XML elements.

  About 4KB smaller in decoded size for JS parsing.
  About 2KB smaller in transfer size.

* Optimise runtime of first `$()` call by removing obsolete support.
* Optimise manipulation methods like `append()`.
* Optimise focus events by using native events when possible/reliable.

Highlights from
https://blog.jquery.com/2022/12/13/jquery-3-6-2-released/
https://blog.jquery.com/2022/12/20/jquery-3-6-3-released-a-quick-selector-fix/
https://blog.jquery.com/2023/03/08/jquery-3-6-4-released-selector-forgiveness
https://blog.jquery.com/2023/05/11/jquery-3-7-0-released-staying-in-order/
https://blog.jquery.com/2023/08/28/jquery-3-7-1-released-reliable-table-row-dimensions/

Change-Id: I144069ce80127692b5c0ff91c4e55383f4013e2f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants