[Webkit-unassigned] [Bug 69605] CSS Counters have wrong values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69605


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #110102|review?                     |review-
               Flag|                            |




--- Comment #2 from mitz at webkit.org  2011-10-07 10:03:14 PST ---
(From update of attachment 110102)
View in context: https://bugs.webkit.org/attachment.cgi?id=110102&action=review

The general seems correct, but the I am not sure about the execution.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:803
> +RenderObject* RenderBoxModelObject::rendererOfAfterPseudoElement()

This should return a RenderBoxModelObject*

> Source/WebCore/rendering/RenderBoxModelObject.cpp:806
> +    while (RenderObject* continuation = static_cast<RenderBoxModelObject*>(lastContinuation)->continuation())

The correct way to write this cast is using the function toRenderBoxModelObject. But there is no reason to do so here, since lastContinuation can be a RenderBoxModelObject* anyway.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:810
> +

Instead of a virtual RenderObject member function, this could be a non-virtual function using virtualContinuation(). It should also start by checking if the document uses before/after rules.

> Source/WebCore/rendering/RenderBoxModelObject.h:125
> +    // This function goes through continuations if any to retrieve the "after"
> +    // pseudoElement.

This comment can fit on one line. The term is “pseudo-element”.

> Source/WebCore/rendering/RenderObject.h:161
> +    // This function only returns the renderer of the "after" pseudoElement
> +    // If it is a virtualChild of this renderer. If "continuations" exist,
> +    // the function returns 0 even if the element that generated this renderer
> +    // has an "after" pseudoElement.

There’s som weird terminology in this comment. There is no such thing as “virtualChild”, and “pseudoElement” should be written “pseudo-element”.

> Source/WebCore/rendering/RenderObject.h:170
> +    // This function goes through continuations if any to retrieve the "after"
> +    // pseudoElement.

This comment can fit on one line.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.