Bug 71000 - Match allowed CSS string characters to Firefox and Opera browsers
Summary: Match allowed CSS string characters to Firefox and Opera browsers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zoltan Herczeg
URL:
Keywords:
Depends on:
Blocks: 69083
  Show dependency treegraph
 
Reported: 2011-10-27 02:01 PDT by Zoltan Herczeg
Modified: 2011-11-02 14:08 PDT (History)
4 users (show)

See Also:


Attachments
patch (6.81 KB, patch)
2011-10-27 02:13 PDT, Zoltan Herczeg
no flags Details | Formatted Diff | Diff
patch2 (6.78 KB, patch)
2011-11-02 02:02 PDT, Zoltan Herczeg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Herczeg 2011-10-27 02:01:05 PDT
Firefox and Opera accepts string characters as in: http://www.w3.org/TR/CSS2/grammar.html

string1		\"([^\n\r\f\\"]|\\{nl}|{escape})*\"
string2		\'([^\n\r\f\\']|\\{nl}|{escape})*\'

Basically everything except newline and starting quote.
Comment 1 Zoltan Herczeg 2011-10-27 02:13:20 PDT
Created attachment 112654 [details]
patch
Comment 2 Darin Adler 2011-10-27 10:35:55 PDT
Comment on attachment 112654 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112654&action=review

> Source/WebCore/css/tokenizer.flex:16
> +string1         \"([^\n\r\f\\"]|\\{nl}|{nonascii}|{escape})*\"
> +string2         \'([^\n\r\f\\']|\\{nl}|{nonascii}|{escape})*\'

You shouldn’t need {nonascii} any more since you are using a negated character class.

Did you research where the set of characters in the current tokenizer came from?
Comment 3 Zoltan Herczeg 2011-10-28 03:13:36 PDT
> Did you research where the set of characters in the current tokenizer came from?

http://trac.webkit.org/changeset/3695 (9 years ago by Hyatt, reviewed by darin/gramps)
Comment 4 Darin Adler 2011-10-28 10:35:46 PDT
(In reply to comment #3)
> > Did you research where the set of characters in the current tokenizer came from?
> 
> http://trac.webkit.org/changeset/3695 (9 years ago by Hyatt, reviewed by darin/gramps)

Hyatt checked it in, but he didn’t write it. The giveaway is all the “konq” prefixed things. This presumably was a check-in of code developed in the KDE repository as part of KHTML, so we’d have to look there.
Comment 5 Zoltan Herczeg 2011-10-28 11:25:59 PDT
> Hyatt checked it in, but he didn’t write it. The giveaway is all the “konq” prefixed things. This presumably was a check-in of code developed in the KDE repository as part of KHTML, so we’d have to look there.

I don't think it is possible now.

https://projects.kde.org/projects/kde/kdelibs/repository/revisions/master/changes/khtml/css/tokenizer.flex

The oldest entry has already contained this rule. And I couldn't find an SVN repository, probably dropped.
Comment 6 Zoltan Herczeg 2011-11-02 02:02:54 PDT
Created attachment 113291 [details]
patch2
Comment 7 WebKit Review Bot 2011-11-02 11:53:24 PDT
Comment on attachment 113291 [details]
patch2

Clearing flags on attachment: 113291

Committed r99086: <http://trac.webkit.org/changeset/99086>
Comment 8 WebKit Review Bot 2011-11-02 11:53:28 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2011-11-02 14:08:50 PDT
This should have fixed some CSS 2.1 test suite tests, I presume? Did it? See bug 47141 for an incomplete list of 2.1 issues.