[Webkit-unassigned] [Bug 70107] Custom written CSS lexer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=70107





--- Comment #33 from Zoltan Herczeg <zherczeg at webkit.org>  2012-01-02 02:19:36 PST ---
Hey Benjamin,

thank you for the review and Happy New Year!

> A general concern is lot of code have the assumption the input is ASCII. Is there no need to handle unicode (identifiers, space, digits, etc?).

Yes. In CSS everything > 127 is called "nonascii" and treated as part of an identifier.

> > Source/WebCore/css/CSSParser.cpp:8032
> > +    // This is not a real loop, just allows us to avoid gotos.
> > +    while (true) {
> 
> Where would the Goto be? I only see one break at the end. The body is a switch().

It is for /* */ comments. See "case CharacterSlash". Although that is the only place, so we could change that to a goto if you prefer that.

> 
> > Source/WebCore/css/CSSParser.cpp:8098
> > +            if (!isASCIIDigit(m_data[0]))
> > +                break;
> 
> Isn't this redundant with the following code?
> The code of CharacterNumber also handle the dot case.

A little more difficult. A single '.' is a dot token, while '.0' is a number (zero in this example).

> 
> > Source/WebCore/css/CSSParser.cpp:8102
> > +            bool dotSeen = (m_token == '.');
> 
> I haven't read the purpose of this yet but that but I have only seen m_token used with named contants everywhere else.

All m_token < 256 are single character tokens. CSSGrammar.y does not define named constant for dot but I can define one if you think it is better.

I'll fix the unlikely issue.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.