[webkit-reviews] review denied: [Bug 22478] Incorrect timer firing in caret blinking. : [Attachment 25472] Handle a non-blinking interval.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Dean McNamee <deanm at chromium.org>'s
request for review:
Bug 22478: Incorrect timer firing in caret blinking.
https://bugs.webkit.org/show_bug.cgi?id=22478

Attachment 25472: Handle a non-blinking interval.
https://bugs.webkit.org/attachment.cgi?id=25472&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I'm not sure why you're renaming the method.  Doesn't really matter to me so
long as all callers and implementers were successfully renamed.

Tabs in your changelog:
+				Renamed caretBlinkFrequency to
caretBlinkInterval.

The description in the bug was much nicer than the one in the Changelog,
perhaps you write something similar in the ChangeLog itself, and make sure to
reference this bug number in the ChangeLog.

WebKit Style:
http://webkit.org/coding/coding-style.html
has no { } around single-line ifs:
+	 if (float blinkInterval = theme()->caretBlinkInterval()) {
+	     d->m_caretBlinkTimer.startRepeating(blinkInterval);
+	 }

Thanks for the patch.  It's hard for me to tell if this is right or not.  Is it
possible to have a non-blinking caret on Windows as well?  Will that cause a
"blink/timer storm" like the Gtk setting will?