[css-syntax] Correctly handle "escaped EOF" #3182

tabatkins · 2018-09-28T21:29:12Z

Per our June 2013 resolution (recorded in #3174), the correct way to handle an "escaped EOF" (that is, a stylesheet ending in a \) is to produce a U+FFFD, except in strings, where it is ignored.

Currently, Syntax correctly specifies that you ignore an escaped EOF in a string, but for anything else, it ends the current token, then reconsumes the \ as a fresh token, which ends up producing a DELIM token containing \.

It looks like I can't just simply modify that clause in "Consume a token", because implementations don't end the preceding token.

Test case:

<!DOCTYPE html>
<style>.foo {--foo:foo\</style>
<style>.foo {--foo:foo \</style>
<style>.foo {--foo:"foo\</style>
<script>
for(var sheet of document.styleSheets) {
  w(sheet.cssRules[0].cssText);
}
</script>

The text was updated successfully, but these errors were encountered:

tabatkins · 2018-10-01T18:47:20Z

Current behavior of the testcase:

Chrome

log: .foo { --foo:foo�; }
log: .foo { --foo:foo �; }
log: .foo { --foo:"foo"; }

Safari

log: .foo { --foo: foo�; }
log: .foo { --foo: foo �; }
log: .foo { --foo: "foo"; }

Firefox

log: .foo { --foo:foo\�; }
log: .foo { --foo:foo \�; }
log: .foo { --foo:"foo"; }

Edge

log: .foo { --foo:foo\; }
log: .foo { --foo:foo \; }
log: .foo { --foo:“foo\; }

(Yes, Edge really printed these, every single one of which isn't even remotely correct.)

So, Edge is clearly super-wrong. Chrome and Safari agree, and obey the old resolution. (How they did so, when the spec doesn't say to do that, I'm not sure...) Firefox obeys the resolution, but unnecessarily escapes the U+FFFD when serializing.

Darn, I was hoping somebody implemented the thing I already had specced, so I didn't have to change anything. ^_^

tabatkins · 2018-10-02T23:02:29Z

...huh. Plugging the first test-case into the dingus, I get the same result as Chrome/WebKit (a U+FFFD that doesn't interrupt the token). This suggests that I changed the spec at some point to the current text. I'll have to research why I did this.

tabatkins · 2018-10-03T15:47:03Z

Ah, found it: #1821

I was trying to fix #-\EOF getting recognized as an ident, but looking at impls, they do indeed treat that as an ident type. (The dingus doesn't, so that's a bug...) I just need to revert that "fix".

tabatkins · 2019-01-17T23:59:04Z

Agenda+ to verify I can fix this. The fix is that an "escaped EOF" (aka a \ at the end of a stylesheet) should return U+FFFD, not a \ as the spec currently mandates. (Except in strings, where it's just silently ignored.) This matches all browsers now that Edge is going away - the only difference is that Firefox serializes such an EOF differently than Chrome/WebKit. (But that's not a Syntax problem, it's a CSSOM problem.)

A side effect of this is that ending a stylesheet with -\EOF will create an ident. This matches browser behavior, so it should be fine.

css-meeting-bot · 2019-01-23T17:07:14Z

The CSS Working Group just discussed Correctly handle "escaped EOF", and agreed to the following:

RESOLVED: Change syntax to specify behavior impl in majority of browsers

The full IRC log of that discussion

<dael> Topic: Correctly handle "escaped EOF"
<dael> github: https://github.com//issues/3182#issuecomment-455376948
<dael> TabAtkins: The spec for syntax- this is a weird corner issue and I'm going to match impl. Turns out the way impl handle an escaped end of file is to omit a replacement character
<dael> TabAtkins: Resolved this at one point, but resolutions are confusing. Spec doesn't say that right now. A slash at the end of the file doesn't create an escape as per spec. FF, Chrome, and Safari think it counts as an escape for replaced character. Want to edt spec to match impl.
<dael> TabAtkins: Edge does extremely wrong things for the slash so I don't think we can draw from that.
<TabAtkins> https://github.com//issues/3182#issuecomment-426019418
<dael> TabAtkins: This is what they do ^
<dael> TabAtkins: I can't tell what they're doing internally, the serialized isn't round trip-able
<dael> TabAtkins: Only difference in other browsers is FF serializes it as an escape for itself which is same result as omitting. It's a CSSOM serialization issue. syntax-relevent has all browsers behaving the same
<dael> astearns: Concerns?
<dael> Rossen: I haven't had a chance to look. Reading through TabAtkins 's comments. I won't push back, but I'm curious what we're doing. I'll comment if I want to re-open.
<dael> astearns: You're fine resolution now?
<dael> Rossen: Yep. Just like any other issue
<dael> astearns: Objections to changing syntax to spec behavior impl in majority of browsers?
<dael> RESOLVED: Change syntax to specify behavior impl in majority of browsers

emilio · 2019-01-23T17:07:31Z

It would be amazing to add a WPT for this if we specify it.

tabatkins · 2019-01-23T17:10:00Z

Yup def. I pushed a few updates last week when closing out issues that I need to write WPTs for too; I'll do that this week.

(For that matter, I think y'all's serialization implementation is wrong here, and will cause you to spuriously fail the test I would write for this issue.)

emilio · 2019-01-23T20:51:26Z

Hmm, why? The serialization roundtrips, doesn't it? cc @SimonSapin

tabatkins · 2019-01-24T22:36:56Z

It roundtrips, yeah, but the escape is unnecessary. Everything outside of ASCII range is fine to be emitted directly; escaping it is basically a no-op.

There's nothing in CSSOM's serialization rules that say to escape that character.

Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821>

tabatkins · 2019-01-26T00:15:27Z

All right, fix committed in spec, and test will be committed as soon as Travis cycles.

@domenic

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per @domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092

@domenic

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per @domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092

@domenic

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per @domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092

@domenic

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per @domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 899781fedf2c2dbc671c092a82bd6928af567075

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 21afd416b4906b2b0ff3745ad10cde023c36c46b

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 899781fedf2c2dbc671c092a82bd6928af567075

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 21afd416b4906b2b0ff3745ad10cde023c36c46b

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 899781fedf2c2dbc671c092a82bd6928af567075

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092 UltraBlame original commit: 21afd416b4906b2b0ff3745ad10cde023c36c46b

@domenic

Automatic update from web-platform-tests Escaped EOF Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821> -- Use assert_throws, etc Per @domenic's advice, go ahead and use assert_throws on the throwing call, and then just make the should-succeed call directly, as a throw will cause the test to fail anyway. -- Merge pull request #15092 from web-platform-tests/tabatkins-patch-1 Escaped EOF -- wpt-commits: 26f39eb230185a804fabc398810cdcdddefc49ac, f44998b147858a8a2832f66dd32bc9d1ecefb1b3, fc57d7ad7a2942cfa363d5a9b8c6c7d74f5bb693 wpt-pr: 15092

tabatkins added the css-syntax-3 label Oct 2, 2018

tabatkins added the Agenda+ label Jan 18, 2019

css-meeting-bot removed the Agenda+ label Jan 23, 2019

tabatkins mentioned this issue Jan 24, 2019

[css-syntax-3] <hash-token> seems to get type ID for sequence "#-\" followed by EOF #1821

Closed

tabatkins closed this as completed in 3bbe9fc Jan 25, 2019

tabatkins added a commit to web-platform-tests/wpt that referenced this issue Jan 26, 2019

Escaped EOF

26f39eb

Tests <w3c/csswg-drafts#3182> and <w3c/csswg-drafts#1821>

tabatkins mentioned this issue Jan 26, 2019

Escaped EOF web-platform-tests/wpt#15092

Merged

tabatkins added Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. Tested Memory aid - issue has WPT tests labels Jan 26, 2019

tabatkins added this to the CSS Syntax 3 June 2019 CR milestone Jun 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[css-syntax] Correctly handle "escaped EOF" #3182

[css-syntax] Correctly handle "escaped EOF" #3182

tabatkins commented Sep 28, 2018

tabatkins commented Oct 1, 2018

Uh oh!

tabatkins commented Oct 2, 2018

Uh oh!

tabatkins commented Oct 3, 2018

Uh oh!

tabatkins commented Jan 17, 2019

Uh oh!

css-meeting-bot commented Jan 23, 2019

Uh oh!

emilio commented Jan 23, 2019

Uh oh!

tabatkins commented Jan 23, 2019

Uh oh!

emilio commented Jan 23, 2019

Uh oh!

tabatkins commented Jan 24, 2019

Uh oh!

tabatkins commented Jan 26, 2019

Uh oh!

[css-syntax] Correctly handle "escaped EOF" #3182

[css-syntax] Correctly handle "escaped EOF" #3182

Comments

tabatkins commented Sep 28, 2018

tabatkins commented Oct 1, 2018

Chrome

Safari

Firefox

Edge

Uh oh!

tabatkins commented Oct 2, 2018

Uh oh!

tabatkins commented Oct 3, 2018

Uh oh!

tabatkins commented Jan 17, 2019

Uh oh!

css-meeting-bot commented Jan 23, 2019

Uh oh!

emilio commented Jan 23, 2019

Uh oh!

tabatkins commented Jan 23, 2019

Uh oh!

emilio commented Jan 23, 2019

Uh oh!

tabatkins commented Jan 24, 2019

Uh oh!

tabatkins commented Jan 26, 2019

Uh oh!