[webkit-reviews] review granted: [Bug 31723] CSS Counter Nesting still does not work according to the spec. : [Attachment 43740] Proposed Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Carol Szabo <carol.szabo at nokia.com>'s
request for review:
Bug 31723: CSS Counter Nesting still does not work according to the spec.
https://bugs.webkit.org/show_bug.cgi?id=31723

Attachment 43740: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=43740&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +			   // We found a reset counter that is on a renderer
that is a sibling of ours or a parent.
> +			   if (isReset && (currentRenderer->parent() ==
counterOwner->parent())) {

Formatting nit: We normally don't use parentheses in this common type of
expression, and later in this same function this patch does not.

> +		       // CurrentCounter, the counter at the EndSearchRenderer
is not reset.

Missing comma after the word "EndSearchRenderer" in this comment.

> +		       if (!isReset || (currentRenderer->parent() !=
counterOwner->parent())) {

Formatting nit: We normally don't use parentheses in this common type of
expression, and later in this same function this patch does not.

> +			   // We are no longer interested in previous siblings
of the currentRenderer or their children
> +			   // as counters they may have attached cannot be the
previous sibling of the counter we are placing

Missing period here.

> +	   // This function is designed so that the same test is not done twice
in an iteration, except for this one
> +	   // which may be done twice in some cases.

I think this design choice has made the function a little harder to read than
most with more repeated code than I would like to see.