384672 - Escaped newline handling should be limited to strings

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Description

[Eli Friedman]

Take a rule like the following:

background: r\
gb(0,0,0);

Mozilla accepts this as a background declaration, but according to the CSS rules escaped newlines are only ignored in strings.

This does bring about the interesting side question of whether the following:

<style>
.a\
{color: red}
</style>
<div class="a\">a</div>

should result in a red a. (Opera tosses out the whole rule; in IE, the rule matches class "a" following some logic which doesn't seem completely consistent.)

Comment 1

[Eli Friedman]

Answering myself: I'm pretty sure that backslash followed by a newline outside a string should result in a malformed declaration according to the CSS rules.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Are other browsers that handle escaped newlines in strings ignoring them outside of strings, or should we just change the spec to match the implementations?

Comment 3

[Eli Friedman]

IE's usually, but not always, treats it as some sort of parse error. However, it sometimes ignores the backslash if it's at the end of a token.

Opera consistently treats a backslash followed by a newline outside of a string as a parse error.

Safari also seems to treat this case as a parse error; I haven't tested thoroughly, though.

I think we're the only implementation that handles backslash newline escapes outside of strings.

Comment 4

[Gauthier Van Vreckem]

Addendum:
gecko removes improperly the \00000a within the string.

see:
http://samples.msdn.microsoft.com/csstestpages/Chapter_4/escaped-newline-001.htm

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #4)
> Addendum:
> gecko removes improperly the \00000a within the string.

Nothing to do with this bug, and see the comments about white-space defaults on :before and :after on public-css-testsuite and www-style right after Microsoft's tests were contributed. (http://lists.w3.org/Archives/Public/)

Comment 6

[Gérard Talbot]

> backslash followed by a newline outside
> a string should result in a malformed declaration according to the CSS rules

This is also the conclusion we reached here:

[CSS2.1] Clarification on section 4.1.3 and the application of escaped newlines to identifiers
http://lists.w3.org/Archives/Public/www-style/2008Nov/0035.html

Zack Weinberg, Boris Zbarsky and fantasai participated into such discussion.

> DIV
> {
> background-color: gre\
> en;
> }

> There is a note that only FireFox currently supports this behavior, 
> which I believe makes them non-compliant.

"Yeah, the Gecko behavior here is just wrong.  We'll fix it." B. Zbarsky

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The working group discussed this today, and I don't think things were so clear-cut.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1: pass in-string boolean to ParseAndAppendEscape

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2: handle failure of GatherIdent and fix up ParseRef

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 3: introduce failure from ParseAndAppendEscape in the EOF cases, and handle that failure

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 4: only allow escaped newlines within strings (string token and string in url() token)

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 5: fix the remaining todo in test_parse_rule.html, a test error

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529382 [details] [diff] [review]
patch 1: pass in-string boolean to ParseAndAppendEscape

r=me

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529383 [details] [diff] [review]
patch 2: handle failure of GatherIdent and fix up ParseRef

>Handle failure of GatherIdent, which can happen (starting with the next patch) fail

  s/happen //

or otherwise fix the grammar.

r=me with that.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529384 [details] [diff] [review]
patch 3: introduce failure from ParseAndAppendEscape in the EOF cases, and handle that failure

r=me

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529385 [details] [diff] [review]
patch 4: only allow escaped newlines within strings (string token and string in url() token)

Hmm.  Why do you not need to push back ch in the !aInString case?  And won't this tokenize escaped newline as DELIM for the '\\' and then whitespace?

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529386 [details] [diff] [review]
patch 5: fix the remaining todo in test_parse_rule.html, a test error

r=me

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #17)
> Comment on attachment 529385 [details] [diff] [review] [review]
> patch 4: only allow escaped newlines within strings (string token and string in
> url() token)
> 
> Hmm.  Why do you not need to push back ch in the !aInString case?  And won't
> this tokenize escaped newline as DELIM for the '\\' and then whitespace?

Er, right, I do need a Pushback() there; without that I'd fail to tokenize the whitespace.  I'm not sure how the difference is testable, though, since we still generate the delim token (per patch 3), and that should cause any parser caller I can think of to fail.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 4: only allow escaped newlines within strings (string token and string in url() token)

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 529796 [details] [diff] [review]
patch 4: only allow escaped newlines within strings (string token and string in url() token)

r=me

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/mozilla-central/rev/c7aa244b92af
https://hg.mozilla.org/mozilla-central/rev/16da10dde7a5
https://hg.mozilla.org/mozilla-central/rev/5a70248eb4ac
https://hg.mozilla.org/mozilla-central/rev/4dd57526d5f6
https://hg.mozilla.org/mozilla-central/rev/fde62ad9cd75