Last Comment Bug 384672 - Escaped newline handling should be limited to strings
: Escaped newline handling should be limited to strings
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P4 normal (vote)
: mozilla6
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
data:text/html,<div style="background...
: 647917 (view as bug list)
Depends on:
Blocks: css2.1-tests
  Show dependency treegraph
 
Reported: 2007-06-15 22:02 PDT by Eli Friedman
Modified: 2011-05-03 13:22 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1: pass in-string boolean to ParseAndAppendEscape (4.00 KB, patch)
2011-05-01 14:26 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 2: handle failure of GatherIdent and fix up ParseRef (7.79 KB, patch)
2011-05-01 14:27 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 3: introduce failure from ParseAndAppendEscape in the EOF cases, and handle that failure (6.89 KB, patch)
2011-05-01 14:27 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 4: only allow escaped newlines within strings (string token and string in url() token) (4.50 KB, patch)
2011-05-01 14:28 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
patch 5: fix the remaining todo in test_parse_rule.html, a test error (1.75 KB, patch)
2011-05-01 14:28 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 4: only allow escaped newlines within strings (string token and string in url() token) (4.52 KB, patch)
2011-05-03 11:57 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review

Description Eli Friedman 2007-06-15 22:02:21 PDT
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 2007-06-15 22:14:21 PDT
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-7 (review requests must explain patch) 2007-06-15 23:48:20 PDT
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 2007-06-16 00:02:29 PDT
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 2008-03-13 13:36:16 PDT
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-7 (review requests must explain patch) 2008-03-13 16:51:34 PDT
(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 2008-11-11 21:33:04 PST
> 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-7 (review requests must explain patch) 2008-11-12 11:14:03 PST
The working group discussed this today, and I don't think things were so clear-cut.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-12 17:52:37 PDT
*** Bug 647917 has been marked as a duplicate of this bug. ***
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-01 14:26:35 PDT
Created attachment 529382 [details] [diff] [review]
patch 1: pass in-string boolean to ParseAndAppendEscape
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-01 14:27:06 PDT
Created attachment 529383 [details] [diff] [review]
patch 2: handle failure of GatherIdent and fix up ParseRef
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-01 14:27:44 PDT
Created attachment 529384 [details] [diff] [review]
patch 3: introduce failure from ParseAndAppendEscape in the EOF cases, and handle that failure
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-01 14:28:15 PDT
Created attachment 529385 [details] [diff] [review]
patch 4: only allow escaped newlines within strings (string token and string in url() token)
Comment 13 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-01 14:28:42 PDT
Created attachment 529386 [details] [diff] [review]
patch 5: fix the remaining todo in test_parse_rule.html, a test error
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-03 09:40:18 PDT
Comment on attachment 529382 [details] [diff] [review]
patch 1: pass in-string boolean to ParseAndAppendEscape

r=me
Comment 15 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-03 10:04:35 PDT
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 [:bz] (Out June 25-July 6) 2011-05-03 10:06:47 PDT
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 [:bz] (Out June 25-July 6) 2011-05-03 10:10:21 PDT
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 [:bz] (Out June 25-July 6) 2011-05-03 10:11:18 PDT
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-7 (review requests must explain patch) 2011-05-03 11:54:59 PDT
(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-7 (review requests must explain patch) 2011-05-03 11:57:09 PDT
Created attachment 529796 [details] [diff] [review]
patch 4: only allow escaped newlines within strings (string token and string in url() token)
Comment 21 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-03 12:08:41 PDT
Comment on attachment 529796 [details] [diff] [review]
patch 4: only allow escaped newlines within strings (string token and string in url() token)

r=me

Note You need to log in before you can comment on or make changes to this bug.