Make tooltip properties idiomatic

The tooltip handling in GtkWidget is "special":

 - the string is stored inside the qdata instead of the private
   instance data
 - the accessors call g_object_set() and g_object_get(), and the
   logic is all inside the property implementation, instead of
   being the other way around
 - the getters return a copy of the string
 - the setters don't really notify all the involved properties

The GtkWidgetAccessible uses the (escaped) tooltip text as a source for
the accessible object description, which means it has to store the
tooltip inside the object qdata, and update its copy at construction and
property notification time.

We can simplify this whole circus by making the tooltip properties (text
and markup) more idiomatic:

 - notify all side-effect properties
 - return a constant string from the getter
 - if tooltip-text is set:
   - store the text as is
   - escape the markup and store it separately for the markup getter
 - if tooltip-markup is set:
   - store the markup as is
   - parse the markup and store it separately for the text getter

The part of the testtooltips interactive test that checks that the
getters are doing the right thing is now part of the gtk testsuite, so
we ensure we don't regress in behaviour.
This commit is contained in:
Emmanuele Bassi 2020-04-22 19:32:17 +01:00
parent 9ac1eacdc8
commit 6b096e5c5b
9 changed files with 204 additions and 132 deletions

View File

@ -26,8 +26,6 @@ struct _GtkWidgetAccessiblePrivate
AtkLayer layer;
};
#define TOOLTIP_KEY "tooltip"
extern GtkWidget *_focus_widget;
@ -90,16 +88,6 @@ map_cb (GtkWidget *widget)
return 1;
}
static void
gtk_widget_accessible_update_tooltip (GtkWidgetAccessible *accessible,
GtkWidget *widget)
{
g_object_set_data_full (G_OBJECT (accessible),
TOOLTIP_KEY,
gtk_widget_get_tooltip_text (widget),
g_free);
}
static void
gtk_widget_accessible_initialize (AtkObject *obj,
gpointer data)
@ -114,11 +102,9 @@ gtk_widget_accessible_initialize (AtkObject *obj,
GTK_WIDGET_ACCESSIBLE (obj)->priv->layer = ATK_LAYER_WIDGET;
obj->role = ATK_ROLE_UNKNOWN;
gtk_widget_accessible_update_tooltip (GTK_WIDGET_ACCESSIBLE (obj), widget);
}
static const gchar *
static const char *
gtk_widget_accessible_get_description (AtkObject *accessible)
{
GtkWidget *widget;
@ -127,10 +113,10 @@ gtk_widget_accessible_get_description (AtkObject *accessible)
if (widget == NULL)
return NULL;
if (accessible->description)
if (accessible->description != NULL)
return accessible->description;
return g_object_get_data (G_OBJECT (accessible), TOOLTIP_KEY);
return gtk_widget_get_tooltip_text (widget);
}
static AtkObject *
@ -469,9 +455,6 @@ gtk_widget_accessible_notify_gtk (GObject *obj,
}
else if (g_strcmp0 (pspec->name, "tooltip-text") == 0)
{
gtk_widget_accessible_update_tooltip (GTK_WIDGET_ACCESSIBLE (atk_obj),
widget);
if (atk_obj->description == NULL)
g_object_notify (G_OBJECT (atk_obj), "accessible-description");
return;

View File

@ -2937,7 +2937,7 @@ static void
ensure_has_tooltip (GtkEntry *entry)
{
GtkEntryPrivate *priv = gtk_entry_get_instance_private (entry);
gchar *text = gtk_widget_get_tooltip_text (GTK_WIDGET (entry));
const char *text = gtk_widget_get_tooltip_text (GTK_WIDGET (entry));
gboolean has_tooltip = text != NULL;
if (!has_tooltip)
@ -2955,10 +2955,6 @@ ensure_has_tooltip (GtkEntry *entry)
}
}
}
else
{
g_free (text);
}
gtk_widget_set_has_tooltip (GTK_WIDGET (entry), has_tooltip);
}

View File

@ -591,7 +591,7 @@ gtk_link_button_query_tooltip_cb (GtkWidget *widget,
{
GtkLinkButton *link_button = GTK_LINK_BUTTON (widget);
const gchar *label, *uri;
gchar *text, *markup;
const char *text, *markup;
label = gtk_button_get_label (GTK_BUTTON (link_button));
uri = link_button->uri;
@ -606,9 +606,6 @@ gtk_link_button_query_tooltip_cb (GtkWidget *widget,
return TRUE;
}
g_free (text);
g_free (markup);
return FALSE;
}

View File

@ -681,7 +681,6 @@ GtkTextDirection gtk_default_direction = GTK_TEXT_DIR_LTR;
static GQuark quark_pango_context = 0;
static GQuark quark_mnemonic_labels = 0;
static GQuark quark_tooltip_markup = 0;
static GQuark quark_size_groups = 0;
static GQuark quark_auto_children = 0;
static GQuark quark_action_muxer = 0;
@ -899,7 +898,6 @@ gtk_widget_class_init (GtkWidgetClass *klass)
quark_pango_context = g_quark_from_static_string ("gtk-pango-context");
quark_mnemonic_labels = g_quark_from_static_string ("gtk-mnemonic-labels");
quark_tooltip_markup = g_quark_from_static_string ("gtk-tooltip-markup");
quark_size_groups = g_quark_from_static_string ("gtk-widget-size-groups");
quark_auto_children = g_quark_from_static_string ("gtk-widget-auto-children");
quark_action_muxer = g_quark_from_static_string ("gtk-widget-action-muxer");
@ -1116,7 +1114,7 @@ gtk_widget_class_init (GtkWidgetClass *klass)
P_("Tooltip Text"),
P_("The contents of the tooltip for this widget"),
NULL,
GTK_PARAM_READWRITE);
GTK_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY);
/**
* GtkWidget:tooltip-markup:
@ -1138,7 +1136,7 @@ gtk_widget_class_init (GtkWidgetClass *klass)
P_("Tooltip markup"),
P_("The contents of the tooltip for this widget"),
NULL,
GTK_PARAM_READWRITE);
GTK_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY);
/**
* GtkWidget:halign:
@ -1684,9 +1682,6 @@ gtk_widget_set_property (GObject *object,
switch (prop_id)
{
gchar *tooltip_markup;
const gchar *tooltip_text;
case PROP_NAME:
gtk_widget_set_name (widget, g_value_get_string (value));
break;
@ -1724,41 +1719,10 @@ gtk_widget_set_property (GObject *object,
gtk_widget_set_has_tooltip (widget, g_value_get_boolean (value));
break;
case PROP_TOOLTIP_MARKUP:
tooltip_markup = g_value_dup_string (value);
/* Treat an empty string as a NULL string,
* because an empty string would be useless for a tooltip:
*/
if (tooltip_markup && (strlen (tooltip_markup) == 0))
{
g_free (tooltip_markup);
tooltip_markup = NULL;
}
g_object_set_qdata_full (object, quark_tooltip_markup,
tooltip_markup, g_free);
gtk_widget_set_has_tooltip (widget, tooltip_markup != NULL);
if (_gtk_widget_get_visible (widget))
gtk_widget_trigger_tooltip_query (widget);
gtk_widget_set_tooltip_markup (widget, g_value_get_string (value));
break;
case PROP_TOOLTIP_TEXT:
tooltip_text = g_value_get_string (value);
/* Treat an empty string as a NULL string,
* because an empty string would be useless for a tooltip:
*/
if (tooltip_text && (strlen (tooltip_text) == 0))
tooltip_text = NULL;
tooltip_markup = tooltip_text ? g_markup_escape_text (tooltip_text, -1) : NULL;
g_object_set_qdata_full (object, quark_tooltip_markup,
tooltip_markup, g_free);
gtk_widget_set_has_tooltip (widget, tooltip_markup != NULL);
if (_gtk_widget_get_visible (widget))
gtk_widget_trigger_tooltip_query (widget);
gtk_widget_set_tooltip_text (widget, g_value_get_string (value));
break;
case PROP_HALIGN:
gtk_widget_set_halign (widget, g_value_get_enum (value));
@ -1883,18 +1847,10 @@ gtk_widget_get_property (GObject *object,
g_value_set_boolean (value, gtk_widget_get_has_tooltip (widget));
break;
case PROP_TOOLTIP_TEXT:
{
gchar *escaped = g_object_get_qdata (object, quark_tooltip_markup);
gchar *text = NULL;
if (escaped && !pango_parse_markup (escaped, -1, 0, NULL, &text, NULL, NULL))
g_assert (NULL == text); /* text should still be NULL in case of markup errors */
g_value_take_string (value, text);
}
g_value_set_string (value, gtk_widget_get_tooltip_text (widget));
break;
case PROP_TOOLTIP_MARKUP:
g_value_set_string (value, g_object_get_qdata (object, quark_tooltip_markup));
g_value_set_string (value, gtk_widget_get_tooltip_markup (widget));
break;
case PROP_HALIGN:
g_value_set_enum (value, gtk_widget_get_halign (widget));
@ -4708,13 +4664,15 @@ gtk_widget_real_query_tooltip (GtkWidget *widget,
gboolean keyboard_tip,
GtkTooltip *tooltip)
{
gchar *tooltip_markup;
const char *tooltip_markup;
gboolean has_tooltip;
tooltip_markup = g_object_get_qdata (G_OBJECT (widget), quark_tooltip_markup);
has_tooltip = gtk_widget_get_has_tooltip (widget);
tooltip_markup = gtk_widget_get_tooltip_markup (widget);
if (tooltip_markup == NULL)
tooltip_markup = gtk_widget_get_tooltip_text (widget);
if (has_tooltip && tooltip_markup)
if (has_tooltip && tooltip_markup != NULL)
{
gtk_tooltip_set_markup (tooltip, tooltip_markup);
return TRUE;
@ -7348,6 +7306,8 @@ gtk_widget_finalize (GObject *object)
gtk_grab_remove (widget);
g_free (priv->name);
g_free (priv->tooltip_markup);
g_free (priv->tooltip_text);
g_clear_object (&priv->accessible);
g_clear_pointer (&priv->transform, gsk_transform_unref);
@ -9650,21 +9610,60 @@ gtk_widget_trigger_tooltip_query (GtkWidget *widget)
/**
* gtk_widget_set_tooltip_text:
* @widget: a #GtkWidget
* @text: (allow-none): the contents of the tooltip for @widget
* @text: (nullable): the contents of the tooltip for @widget
*
* Sets @text as the contents of the tooltip. This function will take
* care of setting #GtkWidget:has-tooltip to %TRUE and of the default
* handler for the #GtkWidget::query-tooltip signal.
* Sets @text as the contents of the tooltip.
*
* See also the #GtkWidget:tooltip-text property and gtk_tooltip_set_text().
* If @text contains any markup, it will be escaped.
*
* This function will take care of setting #GtkWidget:has-tooltip
* as a side effect, and of the default handler for the
* #GtkWidget::query-tooltip signal.
*
* See also the #GtkWidget:tooltip-text property and
* gtk_tooltip_set_text().
*/
void
gtk_widget_set_tooltip_text (GtkWidget *widget,
const gchar *text)
gtk_widget_set_tooltip_text (GtkWidget *widget,
const char *text)
{
GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget);
GObject *object = G_OBJECT (widget);
char *tooltip_text, *tooltip_markup;
g_return_if_fail (GTK_IS_WIDGET (widget));
g_object_set (G_OBJECT (widget), "tooltip-text", text, NULL);
g_object_freeze_notify (object);
/* Treat an empty string as a NULL string,
* because an empty string would be useless for a tooltip:
*/
if (text != NULL && *text == '\0')
{
tooltip_text = NULL;
tooltip_markup = NULL;
}
else
{
tooltip_text = g_strdup (text);
tooltip_markup = text != NULL ? g_markup_escape_text (text, -1) : NULL;
}
g_clear_pointer (&priv->tooltip_markup, g_free);
g_clear_pointer (&priv->tooltip_text, g_free);
priv->tooltip_text = tooltip_text;
priv->tooltip_markup = tooltip_markup;
gtk_widget_set_has_tooltip (widget, priv->tooltip_text != NULL);
if (_gtk_widget_get_visible (widget))
gtk_widget_trigger_tooltip_query (widget);
g_object_notify_by_pspec (object, widget_props[PROP_TOOLTIP_TEXT]);
g_object_notify_by_pspec (object, widget_props[PROP_TOOLTIP_MARKUP]);
g_object_notify_by_pspec (object, widget_props[PROP_HAS_TOOLTIP]);
g_object_thaw_notify (object);
}
/**
@ -9673,63 +9672,99 @@ gtk_widget_set_tooltip_text (GtkWidget *widget,
*
* Gets the contents of the tooltip for @widget.
*
* Returns: (nullable): the tooltip text, or %NULL. You should free the
* returned string with g_free() when done.
* If the @widget's tooltip was set using gtk_widget_set_tooltip_markup(),
* this function will return the escaped text.
*
* Returns: (nullable): the tooltip text
*/
gchar *
const char *
gtk_widget_get_tooltip_text (GtkWidget *widget)
{
gchar *text = NULL;
GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget);
g_return_val_if_fail (GTK_IS_WIDGET (widget), NULL);
g_object_get (G_OBJECT (widget), "tooltip-text", &text, NULL);
return text;
return priv->tooltip_text;
}
/**
* gtk_widget_set_tooltip_markup:
* @widget: a #GtkWidget
* @markup: (allow-none): the contents of the tooltip for @widget, or %NULL
* @markup: (nullable): the contents of the tooltip for @widget
*
* Sets @markup as the contents of the tooltip, which is marked up with
* the [Pango text markup language][PangoMarkupFormat].
* the [Pango text markup language][PangoMarkupFormat].
*
* This function will take care of setting #GtkWidget:has-tooltip to %TRUE
* and of the default handler for the #GtkWidget::query-tooltip signal.
* This function will take care of setting the #GtkWidget:has-tooltip as
* a side effect, and of the default handler for the #GtkWidget::query-tooltip signal.
*
* See also the #GtkWidget:tooltip-markup property and
* gtk_tooltip_set_markup().
*/
void
gtk_widget_set_tooltip_markup (GtkWidget *widget,
const gchar *markup)
gtk_widget_set_tooltip_markup (GtkWidget *widget,
const char *markup)
{
GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget);
GObject *object = G_OBJECT (widget);
char *tooltip_markup;
g_return_if_fail (GTK_IS_WIDGET (widget));
g_object_set (G_OBJECT (widget), "tooltip-markup", markup, NULL);
g_object_freeze_notify (object);
/* Treat an empty string as a NULL string,
* because an empty string would be useless for a tooltip:
*/
if (markup != NULL && *markup == '\0')
tooltip_markup = NULL;
else
tooltip_markup = g_strdup (markup);
g_clear_pointer (&priv->tooltip_text, g_free);
g_clear_pointer (&priv->tooltip_markup, g_free);
priv->tooltip_markup = tooltip_markup;
/* Store the tooltip without markup, as we might end up using
* it for widget descriptions in the accessibility layer
*/
if (priv->tooltip_markup != NULL)
{
pango_parse_markup (priv->tooltip_markup, -1, 0, NULL,
&priv->tooltip_text,
NULL,
NULL);
}
gtk_widget_set_has_tooltip (widget, tooltip_markup != NULL);
if (_gtk_widget_get_visible (widget))
gtk_widget_trigger_tooltip_query (widget);
g_object_notify_by_pspec (object, widget_props[PROP_TOOLTIP_TEXT]);
g_object_notify_by_pspec (object, widget_props[PROP_TOOLTIP_MARKUP]);
g_object_notify_by_pspec (object, widget_props[PROP_HAS_TOOLTIP]);
g_object_thaw_notify (object);
}
/**
* gtk_widget_get_tooltip_markup:
* @widget: a #GtkWidget
*
* Gets the contents of the tooltip for @widget.
* Gets the contents of the tooltip for @widget set using
* gtk_widget_set_tooltip_markup().
*
* Returns: (nullable): the tooltip text, or %NULL. You should free the
* returned string with g_free() when done.
* Returns: (nullable): the tooltip text
*/
gchar *
const char *
gtk_widget_get_tooltip_markup (GtkWidget *widget)
{
gchar *text = NULL;
GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget);
g_return_val_if_fail (GTK_IS_WIDGET (widget), NULL);
g_object_get (G_OBJECT (widget), "tooltip-markup", &text, NULL);
return text;
return priv->tooltip_markup;
}
/**

View File

@ -723,22 +723,22 @@ void gtk_widget_remove_mnemonic_label (GtkWidget *widget,
GtkWidget *label);
GDK_AVAILABLE_IN_ALL
void gtk_widget_trigger_tooltip_query (GtkWidget *widget);
void gtk_widget_trigger_tooltip_query (GtkWidget *widget);
GDK_AVAILABLE_IN_ALL
void gtk_widget_set_tooltip_text (GtkWidget *widget,
const gchar *text);
void gtk_widget_set_tooltip_text (GtkWidget *widget,
const char *text);
GDK_AVAILABLE_IN_ALL
gchar * gtk_widget_get_tooltip_text (GtkWidget *widget);
const char * gtk_widget_get_tooltip_text (GtkWidget *widget);
GDK_AVAILABLE_IN_ALL
void gtk_widget_set_tooltip_markup (GtkWidget *widget,
const gchar *markup);
void gtk_widget_set_tooltip_markup (GtkWidget *widget,
const char *markup);
GDK_AVAILABLE_IN_ALL
gchar * gtk_widget_get_tooltip_markup (GtkWidget *widget);
const char * gtk_widget_get_tooltip_markup (GtkWidget *widget);
GDK_AVAILABLE_IN_ALL
void gtk_widget_set_has_tooltip (GtkWidget *widget,
gboolean has_tooltip);
void gtk_widget_set_has_tooltip (GtkWidget *widget,
gboolean has_tooltip);
GDK_AVAILABLE_IN_ALL
gboolean gtk_widget_get_has_tooltip (GtkWidget *widget);
gboolean gtk_widget_get_has_tooltip (GtkWidget *widget);
GDK_AVAILABLE_IN_ALL
GType gtk_requisition_get_type (void) G_GNUC_CONST;

View File

@ -193,6 +193,10 @@ struct _GtkWidgetPrivate
/* Pointer cursor */
GdkCursor *cursor;
/* Tooltip */
char *tooltip_markup;
char *tooltip_text;
};
typedef struct

View File

@ -293,7 +293,7 @@ main (int argc, char *argv[])
GtkTextIter iter;
GtkTextTag *tag;
gchar *text, *markup;
const char *text, *markup;
gboolean done = FALSE;
gtk_init ();
@ -319,9 +319,8 @@ main (int argc, char *argv[])
text = gtk_widget_get_tooltip_text (button);
markup = gtk_widget_get_tooltip_markup (button);
g_assert (g_str_equal ("Hello, I am a static tooltip.", text));
g_assert (g_str_equal ("Hello, I am a static tooltip.", markup));
g_free (text); g_free (markup);
g_assert_true (g_str_equal ("Hello, I am a static tooltip.", text));
g_assert_true (g_str_equal ("Hello, I am a static tooltip.", markup));
/* A check button using the query-tooltip signal */
button = gtk_check_button_new_with_label ("I use the query-tooltip signal");
@ -338,9 +337,8 @@ main (int argc, char *argv[])
text = gtk_widget_get_tooltip_text (button);
markup = gtk_widget_get_tooltip_markup (button);
g_assert (g_str_equal ("Label & and tooltip", text));
g_assert (g_str_equal ("Label & and tooltip", markup));
g_free (text); g_free (markup);
g_assert_true (g_str_equal ("Label & and tooltip", text));
g_assert_true (g_str_equal ("Label & and tooltip", markup));
/* A selectable label */
button = gtk_label_new ("I am a selectable label");
@ -350,9 +348,8 @@ main (int argc, char *argv[])
text = gtk_widget_get_tooltip_text (button);
markup = gtk_widget_get_tooltip_markup (button);
g_assert (g_str_equal ("Another Label tooltip", text));
g_assert (g_str_equal ("<b>Another</b> Label tooltip", markup));
g_free (text); g_free (markup);
g_assert_true (g_str_equal ("Another Label tooltip", text));
g_assert_true (g_str_equal ("<b>Another</b> Label tooltip", markup));
/* An insensitive button */
button = gtk_button_new_with_label ("This one is insensitive");

View File

@ -62,6 +62,7 @@ tests = [
['textbuffer'],
['textiter'],
['theme-validate'],
['tooltips'],
['treelistmodel'],
['treemodel', ['treemodel.c', 'liststore.c', 'treestore.c', 'filtermodel.c',
'modelrefcount.c', 'sortmodel.c', 'gtktreemodelrefcount.c']],

59
testsuite/gtk/tooltips.c Normal file
View File

@ -0,0 +1,59 @@
/* tooltips.c: Unit test for GtkWidget tooltip accessors
*
* SPDX-License-Identifier: LGPL-2.1-or-later
* Copyright 2020 GNOME Foundation
*/
#include <gtk/gtk.h>
static void
test_tooltips_widget_accessors (void)
{
GtkWidget *w;
const char *text, *markup;
g_test_message ("A button using tooltip-markup");
w = gtk_check_button_new_with_label ("This one uses the tooltip-markup property");
g_object_ref_sink (w);
gtk_widget_set_tooltip_text (w, "Hello, I am a static tooltip.");
text = gtk_widget_get_tooltip_text (w);
markup = gtk_widget_get_tooltip_markup (w);
g_assert_cmpstr (text, ==, "Hello, I am a static tooltip.");
g_assert_cmpstr (markup, ==, "Hello, I am a static tooltip.");
g_object_unref (w);
g_test_message ("A label using tooltip-text");
w = gtk_label_new ("I am just a label");
g_object_ref_sink (w);
gtk_widget_set_tooltip_text (w, "Label & and tooltip");
text = gtk_widget_get_tooltip_text (w);
markup = gtk_widget_get_tooltip_markup (w);
g_assert_cmpstr (text, ==, "Label & and tooltip");
g_assert_cmpstr (markup, ==, "Label &amp; and tooltip");
g_object_unref (w);
g_test_message ("A label using tooltip-markup");
w = gtk_label_new ("I am a selectable label");
g_object_ref_sink (w);
gtk_label_set_selectable (GTK_LABEL (w), TRUE);
gtk_widget_set_tooltip_markup (w, "<b>Another</b> Label tooltip");
text = gtk_widget_get_tooltip_text (w);
markup = gtk_widget_get_tooltip_markup (w);
g_assert_cmpstr (text, ==, "Another Label tooltip");
g_assert_cmpstr (markup, ==,"<b>Another</b> Label tooltip");
g_object_unref (w);
}
int
main (int argc,
char *argv[])
{
gtk_test_init (&argc, &argv);
g_test_add_func ("/tooltips/widget-accessors", test_tooltips_widget_accessors);
return g_test_run ();
}