Bug 31704 – [chromium] Add setCaretBlinkInterval API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Joel Stanley]

With this, the gtk port of chromium will set the caret blink rate.

Chromium side: http://codereview.chromium.org/398003/show

[Joel Stanley]

Created attachment 43547 [details]
patch

[Evan Martin]

(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

[Darin Fisher (:fishd, Google)]

Comment on attachment 43547 [details]
patch

> +++ b/WebKit/chromium/src/WebViewImpl.cpp
...
> +void WebViewImpl::setCaretBlinkInterval(double interval) {
> +#if PLATFORM(LINUX)
> +    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.

-darin

[Joel Stanley]

Created attachment 47924 [details]
updated

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.

[Joel Stanley]

Created attachment 47977 [details]
patch

[David Levin]

Comment on attachment 47977 [details]
patch

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> [phone]  Joel Stanley  <[e-mail]>
> +
> +        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.

[David Levin]

I'll land this and do the fix suggested.

[David Levin]

Committed as http://trac.webkit.org/changeset/54280