[webkit-reviews] review denied: [Bug 6248] CSS3: implement nth-* selectors (Acid3 bug) : [Attachment 16665] Cleaned up patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Mark Rowe (bdash) <mrowe at apple.com>'s
request for review:
Bug 6248: CSS3: implement nth-* selectors (Acid3 bug)
http://bugs.webkit.org/show_bug.cgi?id=6248

Attachment 16665: Cleaned up patch
http://bugs.webkit.org/attachment.cgi?id=16665&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
The change in general looks fine.  The patch needs a style refresh and
changelog.

	     WebCore::Document *doc = p->document();
	     if (doc)
		doc->setUsesSiblingRules(true);

should probably just be if (p->document())
  p->document()->setUsesSiblingRules(true);

Style:
+	 } else {
+	     b = nth.toInt();
+	 }

No else after return:
+	 else
+	     return (b - count) % (-a) == 0;

+static bool matchNth(int count, int a, int b)
actually has a bunch of style issues (a == 0), several else blocks after
returns.  no need for if (a < 0), etc.

I've changed my mind.  I think it's OK to get this basic functionality landed,
and file a second bug to track making it dynamic.

r- for the style issues.