[Webkit-unassigned] [Bug 40327] Leftover calls to RenderStyle color accessors, which are no longer public methods.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jun 16 08:13:00 PDT 2010


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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #58889|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #14 from Darin Adler <darin at apple.com>  2010-06-16 08:13:00 PST ---
(From update of attachment 58889)
> -        RenderBlock* container = new (renderArena()) RenderBlock(node());
> +        RenderBlock* container = new (renderArena()) RenderMathMLBlock(node());

What does this change have to do with the bug you're fixing? This seems OK to change, but unrelated to the change log or other changes.
>          
> -    RenderBlock* container = new (renderArena()) RenderBlock(node());
> +    RenderBlock* container = new (renderArena()) RenderMathMLBlock(node());

Ditto.

> -    toRenderMathMLBlock(current)->style()->setVerticalAlign(BASELINE);
> +    // FIXME: should we really be setting the vertical align here?
> +    current->style()->setVerticalAlign(BASELINE);

Comment sentence should be capitalized. Also, would be better if the comment said why it's questionable. As it is, the comment just says "should this next line of code exist", which doesn't give context for what your doubt is.

> +    // Adjust the bottom position of the index.
>      indexBox->style()->setBottom(Length(radicalHeight + style()->paddingBottom().value(), Fixed));

This comment says the same thing the line of code does. We normally leave those out, and make sure comments say something the code does not say.

Patch otherwise looks fine. review- because of the seemingly unrelated changes

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



More information about the webkit-unassigned mailing list