Summary: | Add new CSS nth-children parsing tests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Herczeg <zherczeg> | ||||
Component: | CSS | Assignee: | 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
Zoltan Herczeg
2011-12-09 06:20:34 PST
Created attachment 118570 [details]
patch
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. > 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.
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. 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 |