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

[bugzilla-daemon at webkit.org]

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





--- Comment #58 from Zoltan Herczeg <zherczeg at webkit.org>  2012-01-25 05:59:25 PST ---
AP ythanks for the in-depth review. I am agree all your name change suggestions.

> > Source/WebCore/css/CSSParser.cpp:7491
> > +    return chr >= 128 || typesOfASCIICharacters[chr] <= CharacterDash;
> 
> Even U+FFFD and unassigned code points are CSS letters?

No they are not letters. Characters > 127 are called non-ASCII in CSS, and they all treated as part of literals regardless what they are.

> > Source/WebCore/css/CSSParser.cpp:7543
> > +        if (isHTMLSpace(*data))
> 
> Are HTML and CSS whitespace definitions the same?

Yes. It is used several other places in the CSS parser.

> 
> > Source/WebCore/css/CSSParser.cpp:7552
> > +    while (*data <= ' ' && (typesOfASCIICharacters[*data] == CharacterWhiteSpace))
> 
> This is a yet another definition of whitespace, with all lower ASCII code being treated as such. If this is right, please explain in a comment.

I changed it to isHTMLSpace.

> > Source/WebCore/css/CSSParser.cpp:7619
> > +inline bool CSSParser::parseIdentifier(UChar*& result)
> 
> Some added "parse" functions are void, but this returns a boolean. This is super confusing. Is there a way to have uniform interface for these? Or names that explain the difference?

The return value is moved to bool& argument

> What is m_data, is it actually m_nextCharacter?

It is the currentCharacter more precisely. I renamed to that.

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