Bug 31704

Summary: [chromium] Add setCaretBlinkInterval API
Product: WebKit Reporter: Joel Stanley <joel>
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: evan, fishd, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Description Flags
fishd: review-
patch levin: review+, levin: commit-queue-

Description Joel Stanley 2009-11-19 21:03:13 PST
With this, the gtk port of chromium will set the caret blink rate.

Chromium side: http://codereview.chromium.org/398003/show
Comment 1 Joel Stanley 2009-11-19 21:05:44 PST
Created attachment 43547 [details]
Comment 2 Evan Martin 2009-11-19 21:21:14 PST
(I am not a reviewer)

The changelog description should probably mention what port it's for.

The comment in WebView.h is probably unnecessary.

LGTM otherwise
Comment 3 Darin Fisher (:fishd, Google) 2009-11-19 21:27:33 PST
Comment on attachment 43547 [details]

> +++ b/WebKit/chromium/src/WebViewImpl.cpp
> +void WebViewImpl::setCaretBlinkInterval(double interval) {
> +    reinterpret_cast<RenderThemeChromiumLinux*>(theme())->setCaretBlinkInterval(interval);
> +#endif
> +}

theme() actually returns a singleton, so mutating this singleton via
a WebView instance is a bit confusing since it would cause the value
to be changed for all WebView instances.

since this is Linux only, perhaps you should just create a linux specific
header file in public/linux that exposes a static setter for this propery.

Comment 4 Joel Stanley 2010-02-02 05:13:30 PST
Created attachment 47924 [details]

Here is another try at this patch.

Note that it doesn't pass check-webkit-style due to unnecessary indentation in WebCore/rendering/RenderThemeChromiumLinux.h, which was preexisting.  I left it as is to minimise noise in the patch.
Comment 5 Joel Stanley 2010-02-02 15:16:46 PST
Created attachment 47977 [details]
Comment 6 David Levin 2010-02-02 16:18:16 PST
Comment on attachment 47977 [details]

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2010-02-02  Joel Stanley  <joel@jms.id.au>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        [Chromium] Make setCaretBlinkInterval static.

This should include a bug link here like you did in the other changelog in this patch.
Comment 7 David Levin 2010-02-02 16:24:17 PST
I'll land this and do the fix suggested.
Comment 8 David Levin 2010-02-03 07:40:00 PST
Committed as http://trac.webkit.org/changeset/54280