csswg-drafts icon indicating copy to clipboard operation
csswg-drafts copied to clipboard

[css-animations] Clarify whether `<keyframes-name>` supports the empty string

Open yisibl opened this issue 3 years ago • 24 comments

The spec allows <string> here https://github.com/w3c/csswg-drafts/issues/118#issuecomment-227655060, but does not further clarify whether empty strings("" or " ") are allowed.

@ewilligers Says:

Also, do we want to ban the empty string '""' ? https://github.com/w3c/csswg-drafts/issues/2435#issuecomment-445630241

  • From the CSS author's point of view, empty strings seem to be meaningless.
  • For current implementations across browsers, this has significant compatibility issues.

@keyframes "" {}

If the browser supports it, the page background color will be green.

@-webkit-keyframes "" {
    to {
        background: green
    }
}

@-moz-keyframes "" {
    to {
        background: green
    }
}

body {
    background: red;
    -webkit-animation: "" 0s ease 1 forwards;
    -moz-animation: "" 0s ease 1 forwards;
}
  • -webkit- (Chrome) ✅
  • -webkit-(Safari) ❌
  • -moz-(Firefox) ❌
  • without prefix(both) ❌ Firefox bug

@keyframes " " {}

@-webkit-keyframes " " {
    to {
        background: green
    }
}

@-moz-keyframes " " {
    to {
        background: green
    }
}

body {
    background: red;
    -webkit-animation: " " 0s ease 1 forwards;
    -moz-animation: " " 0s ease 1 forwards;
}
  • -webkit- (Chrome) ✅
  • -webkit-(Safari) ✅
  • -moz-(Firefox) ✅

yisibl avatar Sep 19 '22 04:09 yisibl

Firefox has a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1554737

yisibl avatar Feb 12 '25 04:02 yisibl

I'm inclined to say that empty string is not allowed, as that cannot be reproduced as an ident. (All other strings can be, with suitable escapes.)

tabatkins avatar Feb 13 '25 01:02 tabatkins

I agree with @tabatkins

fantasai avatar Oct 20 '25 22:10 fantasai

@fantasai is the PR https://github.com/w3c/csswg-drafts/pull/12985 enough to solve this issue ?

javifernandez avatar Oct 20 '25 23:10 javifernandez

@javifernandez After CL 7128859, Chrome will change the behavior of -webkit-. The following test cases will display with a red background instead of the previous green background. I believe this requires adding a WPT test with ref.

As mentioned, Chrome only supports empty string names "" when -webkit- is present. In practice, developers are unlikely to write such code, and relevant tools can provide lint checks or warnings to prevent it.

Therefore, I believe Chrome is moving in the right direction.

data:text/html;charset=UTF-8,
<!doctype html>
<style>
  @-webkit-keyframes "" {
    to {
      background: green
    }
  }
  .test {
   color: white;
   width: 360px;
   height: 50px;
   margin-top: 10px;
   background-color: red;
   animation-fill-mode: forwards;
  }
</style>

<h3>The following should have a green background:</h3>
<div class="test" style="-webkit-animation: '' 0s forwards;">-webkit-animation</div>
<div class="test" style="-webkit-animation-name: '';">-webkit-animation-name</div>

yisibl avatar Nov 10 '25 08:11 yisibl

There are already a test [1] checking that an empty string is invalid for "animation-name".

https://github.com/web-platform-tests/wpt/blob/master/css/css-animations/parsing/animation-name-invalid.html

Do you think we still need ref tests for this case ?

In any case, there aren´t many tests, either testharness or ref tests, for the prefixed property in the WPT repository, AFAIK. There are a many in the blink's and webkit's internal repos, though. We could start migrating them to the WPT repo, if we think they are still useful to ensure back-compatibility .

javifernandez avatar Nov 10 '25 09:11 javifernandez

Hi @dholbert , I'm moving the discussion form PR #12985 here.

Seems to me like we should ensure it's specified. Right now, Chrome Canary/Safari/Firefox all differ on how to serialize animation-name: '' (and specifically whether you get the empty string vs. none in the specified & computed style.) See https://bugzilla.mozilla.org/show_bug.cgi?id=1998933#c6 for testcases and results.

I think there is a confusion here between '' and '""'. The former would clear out the previous specified value, if any, so it has no value (defaulting to the initial value as resolved and computed value). I guess no value is serialized as an empty string and as I said, the computed value would be none as <ident>; this is Firefox's implemented behavior, as far as I know. The latter is considered as an invalid value, so it won't be assigned; hence, the specified value would be the previous one, if any.

This is a web-exposed behavior; the WPT (https://wpt.fyi/results/css/css-animations/parsing/animation-name-invalid.html ) happens to test the specified-style serialization where Chrome/Safari happen to now agree and diverge from Firefox, and the divergence is why Firefox is currently marked as failing a subtest there.

The failure comes from the fact that test_invalid_value assigns '' to the property to clear out the previous value. It seems that Firefox accepts '""' as a valid <string> value, when it shouldn't as agreed here by Tab and Fantasai. Then Firefox serializes the empty string as ````none```, but the lack of inteop comes form the fact that Firefox accepts the empty string as valid value.

Chrome agrees with Firefox on the computed-style-serialization, though; so if there were a WPT for that (maybe there is?) then Chrome/Firefox would get one result and Safari would get another.

The animation-name-computed.html test should be the one to capture such test case. But since the empty string should be invalid, it wouldn't make sense to add it.

javifernandez avatar Nov 11 '25 23:11 javifernandez

Hi @dholbert , I'm moving the discussion form PR #12985 here.

Thanks!

The latter '""' is considered as an invalid value, so it won't be assigned;

This^ does not seem to be accurate (at least in Chrome Canary/Firefox/Safari TP), actually, based on my testcase. See below.

The failure comes from the fact that test_invalid_value assigns '' to the property to clear out the previous value.

I'm not focusing on that assignment inside of test_invalid_value. Rather, I'm focusing specifically on the last part of animation-name-computed.html where it literally tests '""' and expects it to be an invalid value:

test_invalid_value("animation-name", '""');

Currently, Chrome Canary & Safari TP & Firefox Nightly all agree that the value parses successfully. It's just that Chrome and Safari return the empty string as the serialized value, so it's indistinguishable whether they accepted it or rejected it. But you can tell that they accepted it, because it influences the cascade, if you check the computed style.

Here's a testcase to exercise this behavior: https://bug1998933.bmoattachments.org/attachment.cgi?id=9526306

That testcase has a CSS rule in a <style> block to set animation-name to 'someStringValue', and then we use script to try setting animation-name in the inline style attribute, just as the WPT does:

testDiv.style.animationName = '""';

If any browser fully rejected that value as invalid-at-parse-time (as you're saying they do), then we would expect that the testcase would still report 'someStringValue' as the final computed style. (This is indeed what happens in Chrome 143.)

But neither Firefox, Safari, nor Chrome 144 Canary do that -- they all change the computed style (to none in Firefox and Chrome canary, and to the empty string in Safari).

It seems that Firefox accepts '""' as a valid <string> value, when it shouldn't as agreed here by Tab and Fantasai.

Not quite. As shown in my testcase:

  • That^ description (accepting '""' as a valid <string> value) seems to match what Safari does -- they accept the empty string and even use it as the computed value for animation-name.
  • Chrome also accepts the empty string, and serialize to the empty string, but treat it as none by computed-value time.
  • Firefox accepts the empty string and internally treats it as none at parse time (because we literally encode the special none value as the empty-string, see here in the definition of KeyframesName which is the underlying type here.

So, I propose:

  • we need to adjust animation-name-computed.html to not treat '""' as invalid, since (for now at least) all the major browsers do treat it as a parsable value.
  • but we need to decide how it should be handled (at parse time and computed-value time)

dholbert avatar Nov 12 '25 21:11 dholbert

It seems that Firefox accepts '""' as a valid <string> value, when it shouldn't as agreed here by Tab and Fantasai.

Not quite. As shown in my testcase:

  • That^ description (accepting '""' as a valid <string> value) seems to match what Safari does -- they accept the empty string and even use it as the computed value for animation-name.

  • Chrome also accepts the empty string, and serialize to the empty string, but treat it as none by computed-value time.

  • Firefox accepts the empty string and internally treats it as none at parse time (because we literally encode the special none value as the empty-string, see here in the definition of KeyframesName which is the underlying type here.

I'm biased of course :) but personally I think Firefox's behavior (treating a quoted empty-string as an alias for none, effectively) seems most-coherent here.

Chrome Canary's behavior (treating the empty string as a valid value, and serializing it as the empty string in specified styles, but coercing it to "none" in the computed value) seems internally inconsistent.

And Safari's behavior (treating the empty string as fully-valid and usable as an actual computed value) seems broken and seems to be the-most-going-against Tab & Fantasai's preference here.

dholbert avatar Nov 12 '25 22:11 dholbert

Replying to earlier comment https://github.com/w3c/csswg-drafts/issues/7762#issuecomment-3510548776:

There are already a test [1] checking that an empty string is invalid for "animation-name".

https://github.com/web-platform-tests/wpt/blob/master/css/css-animations/parsing/animation-name-invalid.html

Do you think we still need ref tests for this case ?

(Answering this^ question: we probably do need a more-thorough test of some sort (whether a reftest or a computed-style test), since per my comments above, animation-name-invalid.html only checks that the empty string produces an empty specified-style serialization. But in fact all browsers treat it as parsable and as having an effect on the computed style; but Chrome/Safari just serialize it as the empty string in elem.style.)

dholbert avatar Nov 12 '25 22:11 dholbert

I believe at least this kind of WPT testing is necessary.

data:text/html;charset=UTF-8,<!doctype html>
<style>
  @-webkit-keyframes "" {
    to { background: red }
  }
  @keyframes "string" {
    to { background: green }
  }
  @keyframes "none" {
    to { background: green }
  }
  @keyframes "inherit" {
    to { background: green }
  }
  @keyframes "initial" {
    to { background: green }
  }
  @keyframes "revert" {
    to { background: green }
  }
  @keyframes "revert-layer" {
    to { background: green }
  }
  @keyframes "revert-rule" { /* New CSS-wide keyword */
    to { background: green }
  }
  @keyframes "unset" {
    to { background: green }
  }
  
  .test {
    color: white;
    width: 300px;
    height: 50px;
    margin-top: 10px;
    background-color: red;
    animation: 0s both;
  }

  .test-webkit {
    background-color: green;
  }
</style>

<h3>The following should have a green background:</h3>

<div class="test test-webkit" style="-webkit-animation-name: '';"></div>
<div class="test" style="animation-name: 'none';"></div>
<div class="test" style="animation-name: 'inherit';"></div>
<div class="test" style="animation-name: 'initial';"></div>
<div class="test" style="animation-name: 'revert';"></div>
<div class="test" style="animation-name: 'revert-layer';"></div>
<div class="test" style="animation-name: 'revert-rule';"></div>
<div class="test" style="animation-name: 'none'; animation-name: ;">animation-name has no value</div>
Image

yisibl avatar Nov 13 '25 07:11 yisibl

Hi Daniel,

Here's a testcase to exercise this behavior: https://bug1998933.bmoattachments.org/attachment.cgi?id=9526306

That testcase has a CSS rule in a <style> block to set animation-name to 'someStringValue', and then we use script to try setting animation-name in the inline style attribute, just as the WPT does:

testDiv.style.animationName = '""';

If any browser fully rejected that value as invalid-at-parse-time (as you're saying they do), then we would expect that the testcase would still report 'someStringValue' as the final computed style. (This is indeed what happens in Chrome 143.)

But neither Firefox, Safari, nor Chrome 144 Canary do that -- they all change the computed style (to none in Firefox and Chrome canary, and to the empty string in Safari).

Yeah, I check it again and you are right. I had implemented the logic to make the empty string as invalid, but forgot to integrate it because the test was already passing. That's a sing, as you suggested, that we need more-thorough tests for that case.

It seems that Firefox accepts '""' as a valid <string> value, when it shouldn't as agreed here by Tab and Fantasai.

Not quite. As shown in my testcase:

* That^ description (accepting `'""'` as a valid `<string>` value) seems to match what **Safari** does -- they accept the empty string and even use it as the computed value for animation-name.

* Chrome also accepts the empty string, and serialize to the empty string, but treat it as `none` by computed-value time.

* Firefox accepts the empty string and internally treats it as `none` at parse time (because we literally encode the special `none` value as the empty-string, see [here](https://searchfox.org/firefox-main/rev/4fd0fa7e5814c0b51f1dd075821988377bc56cc1/servo/components/style/values/mod.rs#691-693) in the definition of `KeyframesName` which is the underlying type here.

I don't understand well what do you mean with "because we literally encode the special none value as the empty-string"; 'none' is a valid ident for the css-animation-name property, which its serialized as such returning 'none' for both the specified and computed value.

So, I propose:

* we need to adjust `animation-name-computed.html` to not treat `'""'` as invalid, since (for now at least) all the major browsers **do treat it as a parsable value**.

I agree, but if we reach an agreement soon, perhaps it's not worth to change those tests. I agree that we should add more, and the ones suggested in the previous comment could be good candidates.

* but we need to decide how it should be handled (at parse time and computed-value time)

I agree as well, although Tab and Fantasai already expressed opinions to consider it invalid (at parse time, I assume). As such, it wouldn't have any computed value.

That would cover Chrome and Safari's position I guess. Would you agree on behalf of Firefox or do you think we should discuss it again ?

javifernandez avatar Nov 19 '25 12:11 javifernandez

I don't understand well what do you mean with "because we literally encode the special none value as the empty-string" 'none' is a valid ident for the css-animation-name property, which its serialized as such returning 'none' for both the specified and computed value.

I mean that internally, all we store for animation-name is a string -- and Firefox happens to treat the empty-string as a "reserved" sentinel-value which we use to represent the none value.

So when we parse the special none value, we represent that internally as ""; and when we're asked to serialized that value, we notice that it's "" and we produce none as the serialization.

(And also, for now, when we parse an actual empty-string value "" as in the tests here, we represent that internally as "" -- the same representation as for none. So when we're asked to serialize that value, we take the same serialization codepath and produce none as the serialization, since we internally consider the empty-string as having this reserved sentinel-value meaning. Arguably it's a bit broken to have two distinct inputs represented with that same internal value; but it sorta depends on at-what-stage the empty string is considered to be invalid.)

Tab and Fantasai already expressed opinions to consider it invalid (at parse time, I assume). As such, it wouldn't have any computed value. That would cover Chrome and Safari's position I guess. Would you agree on behalf of Firefox or do you think we should discuss it again ?

That seems fine to me; I think that's trivial for Firefox to support, and in fact it would make our sentinel-value representation less-of-a-hack. :) We can just start rejecting the empty string at parse time (while still (ab)using it as our representation for none); I don't think we'd need to change anything else.

(This would be a behavior-change for all browsers, of course -- since, as noted above, all browsers currently do parse the empty string as a valid value here, but just not in a way that this particular WPT is set up to detect. Presumably/hopefully the webcompat impact here should be zero, though. :) )

dholbert avatar Nov 21 '25 21:11 dholbert

I just filed https://bugzilla.mozilla.org/show_bug.cgi?id=2001735 as a Firefox bug on treating the empty string as an invalid specified-value as discussed here. (Not jumping on fixing it right away, as I'm juggling a handful of other things at the moment; but I suspect's an easy fix.)

dholbert avatar Nov 21 '25 22:11 dholbert

(This would be a behavior-change for all browsers, of course -- since, as noted above, all browsers currently do parse the empty string as a valid value here, but just not in a way that this particular WPT is set up to detect. Presumably/hopefully the webcompat impact here should be zero, though. :) )

Yes, it'd be a behavior change, indeed. I have understood from our discussion than some change in the spec would be needed, in order to be more explicit about how to treat the empty string in @keyframe rules.

javifernandez avatar Nov 25 '25 07:11 javifernandez

I have filed https://issues.chromium.org/issues/463591198 in chrome

javifernandez avatar Nov 25 '25 13:11 javifernandez

So, I believe we've reached an asynchronous resolution, correct?

  • RESOLVED: The empty <string> should be parsed as invalid(animation-name: "" would be an invalid value).

@nt1m, what do you think?

yisibl avatar Nov 26 '25 03:11 yisibl

Not opposed to this change, but also not a high value change either :) If we resolve, I will try to find time in between the cracks to implement it.

nt1m avatar Nov 26 '25 10:11 nt1m

Not opposed to this change, but also not a high value change either :) If we resolve, I will try to find time in between the cracks to implement it.

Could you please approve the PR for the spec ?

javifernandez avatar Nov 28 '25 08:11 javifernandez

This is more a formality but this needs a resolution (I think async is probably fine?) cc @astearns

nt1m avatar Nov 28 '25 08:11 nt1m

Could you please clarify if this will also apply to CSSKeyframesRule.name? I think browsers apply the same rule, but I have not checked recently.

cdoublev avatar Nov 28 '25 09:11 cdoublev

Let’s get an answer to @cdoublev’s question above before we go for an async resolution here

astearns avatar Dec 15 '25 20:12 astearns

The grammar for animation-name is defined as

[ none | <keyframes-name> ]#

So indeed I think the change should happen in <keyframes-name>, and thus make both @keyframes "" {} and animation-name: "" invalid.

Loirooriol avatar Dec 15 '25 21:12 Loirooriol

The PR #13151 changes the <keyframes-name> prose to reflect this change, so I guess it follows what it's being concluded.

The tests added in WPT PR #56260 include cases for both animation-name and keyframes-name

javifernandez avatar Dec 16 '25 10:12 javifernandez