Bug 74178

Summary: Add new CSS nth-children parsing tests
Product: WebKit Reporter: Zoltan Herczeg <zherczeg>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, macpherson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 69083    
Attachments:
Description Flags
patch darin: review+, zherczeg: commit-queue-

Description Zoltan Herczeg 2011-12-09 06:20:34 PST
The test covers several valid and invalid nth-child tokens.
Comment 1 Zoltan Herczeg 2011-12-09 06:45:24 PST
Created attachment 118570 [details]
patch
Comment 2 Darin Adler 2011-12-09 09:35:25 PST
Comment on attachment 118570 [details]
patch

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

> Source/WebCore/css/CSSParser.cpp:8092
>      // The tokenizer checks for the construct of an+b.
>      // nth can also accept "n", "odd" or "even" but should not accept any other token.
> -    return equalIgnoringCase(token, "odd") || equalIgnoringCase(token, "even") || equalIgnoringCase(token, "n");
> +    return equalIgnoringCase(token, "odd") || equalIgnoringCase(token, "even")
> +        || equalIgnoringCase(token, "n") || equalIgnoringCase(token, "-n");

The comment now does not match the code. There is no clue here why "-n" is something we need to allow here.
Comment 3 Zoltan Herczeg 2011-12-09 10:04:17 PST
> The comment now does not match the code. There is no clue here why "-n" is something we need to allow here.

Thanks for the review. I'll fix the comment and land manually.
Comment 4 Darin Adler 2011-12-09 10:06:14 PST
When fixing the comment I suggest removing the part that says what the code does, since the code already seems to say that clearly enough, and enhancing the aspect of comment that says *why* the code does what it does.
Comment 5 Zoltan Herczeg 2011-12-11 23:45:53 PST
Changed the comment to:

    // The tokenizer checks for the construct of an+b.
    // However, since the {ident} rule precedes the {nth} rule, some of those
    // tokens are identified as string literal. Furthermore we need to accept
    // "odd" and "even" which does not match to an+b.

And landed: http://trac.webkit.org/changeset/102560