[webkit-reviews] review denied: [Bug 32819] [GTK] Change the tooltip implementation : [Attachment 45378] Proposed patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Alejandro G. Castro
<alex at igalia.com>'s request for review:
Bug 32819: [GTK] Change the tooltip implementation
https://bugs.webkit.org/show_bug.cgi?id=32819

Attachment 45378: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=45378&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
> +#if GTK_CHECK_VERSION(2, 12, 0)
> +    priv->tooltipText = 0;
> +    gtk_widget_set_has_tooltip(GTK_WIDGET(webView), TRUE);
> +    g_signal_connect(webView, "query-tooltip",
G_CALLBACK(webkit_web_view_query_tooltip), 0);
> +#endif

You should override the GtkWidget class method instead of connecting to the
signal.

> +void webkit_web_view_set_tooltip_text(WebKitWebView* webView, const char*
tooltip)
> +{
> +    WebKitWebViewPrivate* priv = webView->priv;
> +    g_free(priv->tooltipText);
> +    if (tooltip && *tooltip != '\0')
> +	   priv->tooltipText = g_strdup(tooltip);
> +    else
> +	   priv->tooltipText = 0;
> +}

What about the 'has-tooltip' property? You should probably set it to
TRUE/FALSE, as appropriate, here. Otherwise the tooltips will only be displayed
when the webview has keyboard focus, as I understand it. The patch looks
awesome otherwise, nice work!