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

whitespace and !important token valid in css custom properties but not in svelte #7152

Closed
tanangular opened this issue Jan 16, 2022 · 5 comments · Fixed by #7303
Closed

whitespace and !important token valid in css custom properties but not in svelte #7152

tanangular opened this issue Jan 16, 2022 · 5 comments · Fixed by #7303

Comments

@tanangular
Copy link

tanangular commented Jan 16, 2022

Describe the bug

h1 {
  --foo: ;
  color: var(--foo) blue;
}
	
h2 {
  --foo: !important;
  color: green var(--foo);
}

When I read this article The CSS Custom Property Toggle Trick, I am trying to use whitespace in css custom properties with svetle, it say "invalid value", but it should be valid according to W3C custom properties specification.

Ref: https://www.w3.org/TR/css-variables-1/#syntax
Screenshot 2565-01-16 at 18 01 19
Screenshot 2565-01-16 at 18 06 53

Reproduction

svelte REPL: https://svelte.dev/repl/d563fcb1d63e405da8a74ae1ef8f30ad?version=3.46.2
codepen: https://codepen.io/tanangular/pen/rNGPgRa

svelte-invalid

whitespace

Logs

No response

System Info

System:
    OS: macOS 12.1
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 360.45 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 17.3.0 - ~/.nvm/versions/node/v17.3.0/bin/node
    npm: 8.3.0 - ~/.nvm/versions/node/v17.3.0/bin/npm
  Browsers:
    Chrome: 97.0.4692.71
    Chrome Canary: 99.0.4828.0
    Edge: 93.0.961.47
    Firefox: 96.0
    Safari: 15.2
    Safari Technology Preview: 15.4
  npmPackages:
    rollup: ^2.3.4 => 2.64.0 
    svelte: ^3.0.0 => 3.46.2

Severity

annoyance

@baseballyama
Copy link
Member

This is just memo but it works well on Firefox.

image

@tanangular
Copy link
Author

@baseballyama, thanks you, you 're right.
This bug has gone on

  • Firefox 96.0.1 ✅
  • Safari 15.2 (17612.3.6.1.6) ✅
  • Safari Technology Preview Release 137 (Safari 15.4, WebKit 17613.1.11.8) ✅
  • Google Chome Canary 99.0.4828.0 ✅

but still have a bug on

  • Google Chrome Version 97.0.4692.71 ❌
  • Microsoft Edge Version 97.0.1072.62 ❌

@geoffrich
Copy link
Member

I think this is a browser bug. Svelte minifies --foo: ; to --foo:;, which should be the same, per the spec. However, Chrome treats --foo:; as invalid. This was recently fixed, and your REPL does work in Chrome Canary (v99).

REPL linked above, with both the headings the correct color

@adamwathan
Copy link

adamwathan commented Feb 19, 2022

Just reposting my comment from #7288 (comment):


Looks like the change to the spec that makes --foo:; and --foo: ; equivalent is quite recent (in terms of spec changes anyways).

Personally I think it would be best to change this in Svelte regardless for better compatibility, it's a change lots of other tools have had to make as well since historically --foo:; and --foo: ; were not equivalent.

This is what the spec said last time I looked a couple of months ago:

Note: While must represent at least one token, that one token may be whitespace. This implies that --foo: ; is valid, and the corresponding var(--foo) call would have a single space as its substitution value, but --foo:; is invalid.

...so I wouldn't be surprised if this sneaky problem shows up a lot for a while. It'll be a while before Electron is on 99 for example I bet.

dummdidumm pushed a commit that referenced this issue Mar 3, 2022
Fixes #7152, see also #7288

--foo:; used to be an invalid CSS custom property value, while -foo: ; was valid. By collapsing the whitespace in these declaration values, we were breaking scenarios where an empty custom property was desired. The spec was updated to trim whitespace and treat these values identically, but Chromium browsers still treat --foo; as invalid. This was recently fixed and will be released in Chrome 99, but this would still be a good fix to maintain backwards compatibility.
himanshiLt pushed a commit to himanshiLt/svelte that referenced this issue Mar 3, 2022
Fixes sveltejs#7152, see also sveltejs#7288

--foo:; used to be an invalid CSS custom property value, while -foo: ; was valid. By collapsing the whitespace in these declaration values, we were breaking scenarios where an empty custom property was desired. The spec was updated to trim whitespace and treat these values identically, but Chromium browsers still treat --foo; as invalid. This was recently fixed and will be released in Chrome 99, but this would still be a good fix to maintain backwards compatibility.
@Conduitry
Copy link
Member

This should be fixed in 3.46.5 for browsers that don't follow the latest spec.

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

Successfully merging a pull request may close this issue.

5 participants