Created attachment 226834 [details] [review]
illustrates problem

Review of attachment 226834 [details] [review]:

there's an issue of attributes coming from the markup: we don't want that, because Pango is unable to parse incomplete markup, and chokes immediately — i.e. you either get the full markup → attributes, or you don't.

GtkEntry got support for attributes recently: you could have a look at how it's been implemented.

::: clutter/clutter-text.c
@@ +502,3 @@
+  /* This will merge the markup attributes and the attributes
+     property if needed */
+  clutter_text_ensure_effective_attributes (text);

instead of merging the attributes between the ones set using set_attributes() and the ones coming from the markup(), you could do something:

  if (!priv->editable)
    clutter_text_ensure_effective_attributes (text);
  else
    {
      if (priv->effective_attrs)
        pango_attr_list_unref (priv->effective_attrs);

      priv->effective_attrs = pango_attr_list_ref (priv->attrs);
    }

or move the check inside clutter_text_ensure_effective_attrs() entirely.

Created attachment 231761 [details] [review]
Allow setting pango attributes to editable text

(In reply to comment #2)
> Review of attachment 226834 [details] [review]:
> 
> there's an issue of attributes coming from the markup: we don't want that,
> because Pango is unable to parse incomplete markup, and chokes immediately —
> i.e. you either get the full markup → attributes, or you don't.
> 
> GtkEntry got support for attributes recently: you could have a look at how it's
> been implemented.
> 

Did you mean this commit? — http://git.gnome.org/browse/gtk+/commit/?id=1ac2982265c87a6164a986b6df161b1af011c944

As you say, it looks like GtkEntry does not merge attributes, and completely ignores markup. I suppose we should do the same here.

> instead of merging the attributes between the ones set using set_attributes()
> and the ones coming from the markup(), you could do something:
> 
[snip]
> or move the check inside clutter_text_ensure_effective_attrs() entirely.

The attached patch does exactly this. Apologies in advance for any screwups — I'm new to this code. :)

Should we mention this quirk in the documentation somewhere? I think developers might get confused when markup doesn't get applied as they expect it to.

Review of attachment 231761 [details] [review]:

looks okay; it would be nice to have a test case in the interactive test suite, or an example.

::: clutter/clutter-text.c
@@ +414,3 @@
 
+  /* There's no attributes, and we ignore markup attributes for editable */
+  if (priv->attrs == NULL && priv->editable)

this condition could be folded into the previous check, for instance:

  if (priv->attrs == NULL && (priv->editable || priv->markup_attrs == NULL))
    return;

(In reply to comment #4)
> Review of attachment 231761 [details] [review]:
> 
> looks okay; it would be nice to have a test case in the interactive test suite,
> or an example.
> 

Would interactive/test-text-field.c do? The test currently adds "<i>some</i> text" to the entry field, which does not work.

> ::: clutter/clutter-text.c
> @@ +414,3 @@
> 
> +  /* There's no attributes, and we ignore markup attributes for editable */
> +  if (priv->attrs == NULL && priv->editable)
> 
> this condition could be folded into the previous check, for instance:
> 
>   if (priv->attrs == NULL && (priv->editable || priv->markup_attrs == NULL))
>     return;

Oops, I don't know how I missed that. Will fix it in the next revision of the patch. :)

Created attachment 232810 [details] [review]
Allow setting pango attributes to editable text + interactive test

thanks for the patch; pushed to the master and clutter-1.14 branches.