Bug 74178 – Add new CSS nth-children parsing tests

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Zoltan Herczeg]

The test covers several valid and invalid nth-child tokens.

[Zoltan Herczeg]

Created attachment 118570 [details]
patch

[Darin Adler]

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.

[Zoltan Herczeg]

> 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.

[Darin Adler]

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.

[Zoltan Herczeg]

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