[clutter] text: Make :use-markup set idempotent

[Emmanuele Bassi <ebassi src gnome org>]

commit 0bd1e47b22de3692c37abaa4582dfb9c802ee493
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Tue Sep 6 12:45:41 2011 +0100

    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

 clutter/clutter-text.c            |   52 +++++++++++++++++----
 tests/conform/test-clutter-text.c |   92 +++++++++++++++++++++++++++++++++++++
 tests/conform/test-conform-main.c |    1 +
 3 files changed, 135 insertions(+), 10 deletions(-)
---
diff --git a/clutter/clutter-text.c b/clutter/clutter-text.c
index 7d65cb2..5423b77 100644
--- a/clutter/clutter-text.c
+++ b/clutter/clutter-text.c
@@ -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;
 
@@ -1028,11 +1031,36 @@ clutter_text_set_positions (ClutterText *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);
 
diff --git a/tests/conform/test-clutter-text.c b/tests/conform/test-clutter-text.c
index ca24fe2..95d6ce8 100644
--- a/tests/conform/test-clutter-text.c
+++ b/tests/conform/test-clutter-text.c
@@ -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));
+}
diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
index d29fc83..39d1fcd 100644
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -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);