[Webkit-unassigned] [Bug 112986] Incorrect error handling for Media Queries

[bugzilla-daemon at webkit.org]

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





--- Comment #14 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2013-03-28 09:00:10 PST ---
(From update of attachment 194516)
View in context: https://bugs.webkit.org/attachment.cgi?id=194516&action=review

> Source/WebCore/ChangeLog:52
> +        * DerivedSources.make:
> +        * GNUmakefile.am:
> +        * css/CSSGrammar.y.in:
> +        * css/CSSParser.cpp:
> +        (WebCore::CSSParser::CSSParser):
> +        (WebCore::CSSParser::parseMediaQuery):
> +        (WebCore::isCSSLetter):
> +        (WebCore::CSSParser::realLex):
> +        (WebCore::CSSParser::rewriteSpecifiers):
> +        * css/CSSParser.h:
> +        (WebCore):
> +        (BracketStack):
> +        (WebCore::BracketStack::push):
> +        (WebCore::BracketStack::pop):
> +        (WebCore::BracketStack::top):
> +        (WebCore::BracketStack::level):
> +        (CSSParser):
> +        (WebCore::CSSParser::reEmitLastToken):
> +        * css/MediaList.cpp:
> +        (WebCore::MediaQuerySet::replaceLastMediaQuery):
> +        (WebCore):
> +        * css/MediaList.h:

You could add some comments here

> Source/WebCore/css/CSSParser.cpp:1490
> -    setupParser("@-webkit-mediaquery ", string, "} ");
> +    // can't use { because tokenizer state switches from mediaquery to initial

Now you are fixing it, please use uppercase C, as comments should start with uppercase.

> Source/WebCore/css/CSSParser.h:507
> +    // Error token reached while parsing a media_query, synchronized at current
> +    // token.

just keep on one line

> Source/WebCore/css/CSSParser.h:670
> +    enum ReEmitState {
> +        NoEmit,
> +        ReEmit,
> +        ReEmitted
> +    };

Why not ReEmissionState { DontEmit, ReEmit, DidReEmit }?

> Source/WebCore/css/CSSParser.h:821
> +inline void CSSParser::reEmitLastToken()
> +{

This method makes it sounds as it is going to re-emit, but that is not really what it is doing

> Source/WebCore/css/CSSParser.h:825
> +    ASSERT(m_reEmitToken == NoEmit);

shouldnt this be != ?

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