text: Make :use-markup set idempotent

Setting :use-markup and :text is currently not idempotent, and it
depends on the ordering, e.g.:

  g_object_set (actor, "use-markup", TRUE, "text", value, NULL);

does not yield the same results as:

  g_object_set (actor, "text", value, "use-markup", TRUE, NULL);

This is particularly jarring when using ClutterText from ClutterScript,
but in general GObject properties should not rely on the order when used
from g_object_set().

The fix is to store the contents of the ClutterText as a separate string
from the displayed text, and use the contents, instead of the displayed
text, when toggling the :use-markup property.

Let's also add a unit test for good measure, to try and catch
regressions.

https://bugzilla.gnome.org/show_bug.cgi?id=651940
This commit is contained in:
Emmanuele Bassi 2011-09-06 12:45:41 +01:00
parent f58c2cec12
commit 0bd1e47b22
3 changed files with 135 additions and 10 deletions

View File

@ -112,6 +112,9 @@ struct _ClutterTextPrivate
{
PangoFontDescription *font_desc;
/* the text passed to set_text and set_markup */
gchar *contents;
/* the displayed text */
gchar *text;
@ -1027,12 +1030,37 @@ clutter_text_set_positions (ClutterText *self,
g_object_thaw_notify (G_OBJECT (self));
}
static inline void
clutter_text_set_contents (ClutterText *self,
const gchar *str)
{
ClutterTextPrivate *priv = self->priv;
g_free (priv->contents);
if (str == NULL || *str == '\0')
priv->contents = g_strdup ("");
else
priv->contents = g_strdup (str);
}
static inline void
clutter_text_set_text_internal (ClutterText *self,
const gchar *text)
{
ClutterTextPrivate *priv = self->priv;
g_signal_emit (self, text_signals[DELETE_TEXT], 0, 0, -1);
if (text != NULL && *text != '\0')
{
gint tmp_pos = 0;
g_signal_emit (self, text_signals[INSERT_TEXT], 0,
text,
strlen (text),
&tmp_pos);
}
g_object_freeze_notify (G_OBJECT (self));
if (priv->max_length > 0)
@ -1154,7 +1182,15 @@ clutter_text_set_property (GObject *gobject,
switch (prop_id)
{
case PROP_TEXT:
clutter_text_set_text_internal (self, g_value_get_string (value));
{
const char *str = g_value_get_string (value);
clutter_text_set_contents (self, str);
if (self->priv->use_markup)
clutter_text_set_markup_internal (self, str);
else
clutter_text_set_text_internal (self, str);
}
break;
case PROP_COLOR:
@ -1422,6 +1458,7 @@ clutter_text_finalize (GObject *gobject)
clutter_text_dirty_paint_volume (self);
g_free (priv->contents);
g_free (priv->text);
g_free (priv->font_name);
@ -3550,6 +3587,7 @@ clutter_text_init (ClutterText *self)
* or strcmp() on it
*/
priv->text = g_strdup ("");
priv->contents = g_strdup ("");
priv->text_color = default_text_color;
priv->cursor_color = default_cursor_color;
@ -4453,15 +4491,8 @@ clutter_text_set_text (ClutterText *self,
{
g_return_if_fail (CLUTTER_IS_TEXT (self));
g_signal_emit (self, text_signals[DELETE_TEXT], 0, 0, -1);
if (text)
{
gint tmp_pos = 0;
g_signal_emit (self, text_signals[INSERT_TEXT], 0, text,
strlen (text), &tmp_pos);
}
clutter_text_set_use_markup_internal (self, FALSE);
clutter_text_set_contents (self, text);
clutter_text_set_text_internal (self, text ? text : "");
}
@ -4490,6 +4521,7 @@ clutter_text_set_markup (ClutterText *self,
g_return_if_fail (CLUTTER_IS_TEXT (self));
clutter_text_set_use_markup_internal (self, TRUE);
clutter_text_set_contents (self, markup);
if (markup != NULL && *markup != '\0')
clutter_text_set_markup_internal (self, markup);
@ -4883,7 +4915,7 @@ clutter_text_set_use_markup (ClutterText *self,
priv = self->priv;
str = g_strdup (priv->text);
str = g_strdup (priv->contents);
clutter_text_set_use_markup_internal (self, setting);

View File

@ -410,3 +410,95 @@ text_event (void)
clutter_actor_destroy (CLUTTER_ACTOR (text));
}
static inline void
validate_markup_attributes (ClutterText *text,
PangoAttrType attr_type,
int start_index,
int end_index)
{
PangoLayout *layout;
PangoAttrList *attrs;
PangoAttrIterator *iter;
layout = clutter_text_get_layout (text);
g_assert (layout != NULL);
attrs = pango_layout_get_attributes (layout);
g_assert (attrs != NULL);
iter = pango_attr_list_get_iterator (attrs);
while (pango_attr_iterator_next (iter))
{
GSList *attributes = pango_attr_iterator_get_attrs (iter);
PangoAttribute *a;
if (attributes == NULL)
break;
g_assert (attributes->data != NULL);
a = attributes->data;
g_assert (a->klass->type == attr_type);
g_assert_cmpint (a->start_index, ==, start_index);
g_assert_cmpint (a->end_index, ==, end_index);
g_slist_free_full (attributes, (GDestroyNotify) pango_attribute_destroy);
}
pango_attr_iterator_destroy (iter);
}
void
idempotent_use_markup (void)
{
ClutterText *text;
const char *contents = "foo <b>bar</b>";
const char *display = "foo bar";
int bar_start_index = strstr (display, "bar") - display;
int bar_end_index = bar_start_index + strlen ("bar");
/* case 1: text -> use_markup */
if (g_test_verbose ())
g_print ("text: '%s' -> use-markup: TRUE\n", contents);
text = g_object_new (CLUTTER_TYPE_TEXT,
"text", contents, "use-markup", TRUE,
NULL);
if (g_test_verbose ())
g_print ("Contents: '%s' (expected: '%s')\n",
clutter_text_get_text (text),
display);
g_assert_cmpstr (clutter_text_get_text (text), ==, display);
validate_markup_attributes (text,
PANGO_ATTR_WEIGHT,
bar_start_index,
bar_end_index);
clutter_actor_destroy (CLUTTER_ACTOR (text));
/* case 2: use_markup -> text */
if (g_test_verbose ())
g_print ("use-markup: TRUE -> text: '%s'\n", contents);
text = g_object_new (CLUTTER_TYPE_TEXT,
"use-markup", TRUE, "text", contents,
NULL);
if (g_test_verbose ())
g_print ("Contents: '%s' (expected: '%s')\n",
clutter_text_get_text (text),
display);
g_assert_cmpstr (clutter_text_get_text (text), ==, display);
validate_markup_attributes (text,
PANGO_ATTR_WEIGHT,
bar_start_index,
bar_end_index);
clutter_actor_destroy (CLUTTER_ACTOR (text));
}

View File

@ -162,6 +162,7 @@ main (int argc, char **argv)
TEST_CONFORM_SIMPLE ("/text", text_get_chars);
TEST_CONFORM_SIMPLE ("/text", text_cache);
TEST_CONFORM_SIMPLE ("/text", text_password_char);
TEST_CONFORM_SIMPLE ("/text", idempotent_use_markup);
TEST_CONFORM_SIMPLE ("/rectangle", test_rect_set_size);
TEST_CONFORM_SIMPLE ("/rectangle", test_rect_set_color);