[webkit-reviews] review granted: [Bug 117347] [rendering] Use foreground color to render the overtype caret : [Attachment 204042] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Claudio Saavedra
<csaavedra at igalia.com>'s request for review:
Bug 117347: [rendering] Use foreground color to render the overtype caret
https://bugs.webkit.org/show_bug.cgi?id=117347

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=204042&action=review


> Source/WebCore/rendering/RenderObject.cpp:1640
> +		   color = frame()->selection()->isFocusedAndActive() ?
>		       theme()->activeSelectionBackgroundColor() :
>		       theme()->inactiveSelectionBackgroundColor();

WebKit style guide says to put the "?" and ":" at the beginnings of lines, not
the ends of lines, or this fits fine on one line, I think.

> Source/WebCore/rendering/RenderView.h:344
> +    bool m_selectionWasCaret;

It would be better to initialize this when in the RenderView constructor, even
if the code will work OK with an uninitialized boolean. If nothing else, the
uninitialized read it will irritate people using memory tools like valgrind.