After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 654015 - empathy-chat asserts with adium themes in tp_escape_as_identifier
empathy-chat asserts with adium themes in tp_escape_as_identifier
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat themes
3.1.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on:
Blocks:
 
 
Reported: 2011-07-05 14:38 UTC by Emilio Pozuelo Monfort
Modified: 2011-07-08 11:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
theme_adium_append_message: cope with tp_message_get_token() returning NULL (1.29 KB, patch)
2011-07-05 15:10 UTC, Guillaume Desmottes
none Details | Review
theme-adium: use the pending-message-id instead of message-token in x-empathy-message-id (4.23 KB, patch)
2011-07-06 14:00 UTC, Guillaume Desmottes
committed Details | Review

Description Emilio Pozuelo Monfort 2011-07-05 14:38:15 UTC
Sending messages in some CMs cause empathy-chat to spit assertions like the one below. The cause is that in empathy-theme-adium.c we do:

	tp_msg = empathy_message_get_tp_message (msg);
	if (tp_msg != NULL) {
		gchar *tmp = tp_escape_as_identifier (
		    tp_message_get_token (tp_msg));
		g_string_append_printf (message_classes,
		    " x-empathy-message-id-%s", tmp);
		g_free (tmp);
	}

tp_message_get_token() may return NULL, and we pass that unconditionally to tp_escape_as_identifier(), which doesn't accept NULL.

tp-glib-CRITICAL **: tp_escape_as_identifier: assertion `name != NULL' failed
aborting...

Program received signal SIGABRT, Aborted.
0x00007fffef839405 in raise (sig=<value optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
64	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
	in ../nptl/sysdeps/unix/sysv/linux/raise.c
(gdb) bt
  • #0 raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 64
  • #1 abort
    at abort.c line 92
  • #2 g_logv
    at /tmp/buildd/glib2.0-2.28.6/./glib/gmessages.c line 557
  • #3 g_log
  • #4 tp_escape_as_identifier
    at util.c line 734
  • #5 theme_adium_append_message
    at empathy-theme-adium.c line 995
  • #6 chat_message_received
    at empathy-chat.c line 1353
  • #7 g_closure_invoke
  • #8 signal_emit_unlocked_R
    at /tmp/buildd/glib2.0-2.28.6/./gobject/gsignal.c line 3252
  • #9 g_signal_emit_valist
    at /tmp/buildd/glib2.0-2.28.6/./gobject/gsignal.c line 2983
  • #10 g_signal_emit
    at /tmp/buildd/glib2.0-2.28.6/./gobject/gsignal.c line 3040
  • #11 tp_chat_emit_queued_messages
    at empathy-tp-chat.c line 271
  • #12 get_contact_by_handle_cb
  • #13 contacts_context_continue
    at contact.c line 1783
  • #14 contacts_context_continue
    at contact.c line 1758
  • #15 contacts_context_idle_continue
    at contact.c line 1824
  • #16 g_main_dispatch
    at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c line 2440
  • #17 g_main_context_dispatch
    at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c line 3013
  • #18 g_main_context_iterate
    at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c line 3091
  • #19 g_main_loop_run
    at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c line 3299
  • #20 gtk_main
    at /tmp/buildd/gtk+3.0-3.0.10/./gtk/gtkmain.c line 1358
  • #21 g_application_run
    at /tmp/buildd/glib2.0-2.28.6/./gio/gapplication.c line 1322
  • #22 main
    at empathy-chat.c line 160

Comment 1 Guillaume Desmottes 2011-07-05 15:10:16 UTC
Created attachment 191327 [details] [review]
theme_adium_append_message: cope with tp_message_get_token() returning NULL
Comment 2 Emilio Pozuelo Monfort 2011-07-05 15:41:48 UTC
+		const gchar *token = tp_message_get_token (tp_msg);
+
+		if (token != NULL) {
+			gchar *tmp = tp_escape_as_identifier (
+			    tp_message_get_token (tp_msg));

That could be

gchar *tmp = tp_escape_as_identifier (token);

theme_adium_remove_mark_from_message() has the same issue (it's passed a token which comes from tp_message_get_token (tp_msg) in theme_adium_message_acknowledged().

What are the consequences of not adding x-empathy-message-id-* ? Will we fail to mark messages as read? If so maybe we shouldn't add the focus classes? And we probably want to implement message-token whenever possible in every CM.
Comment 3 Guillaume Desmottes 2011-07-06 08:23:46 UTC
(In reply to comment #2)
> +        const gchar *token = tp_message_get_token (tp_msg);
> +
> +        if (token != NULL) {
> +            gchar *tmp = tp_escape_as_identifier (
> +                tp_message_get_token (tp_msg));
> 
> That could be
> 
> gchar *tmp = tp_escape_as_identifier (token);

Humm that's what I meant to write, I plan the last patch of the day. :p

> theme_adium_remove_mark_from_message() has the same issue (it's passed a token
> which comes from tp_message_get_token (tp_msg) in
> theme_adium_message_acknowledged().
> 
> What are the consequences of not adding x-empathy-message-id-* ? Will we fail
> to mark messages as read? If so maybe we shouldn't add the focus classes? And
> we probably want to implement message-token whenever possible in every CM.

Humm I thought this token was used for message editing but it's actually use
for acking indeed.
Jonny: shouldn't we use pending-message-id instead of message-token? The
former is guaranteed to be present on all incoming messages while the later is
not.
Comment 4 Guillaume Desmottes 2011-07-06 14:00:21 UTC
Created attachment 191399 [details] [review]
theme-adium: use the pending-message-id instead of message-token in x-empathy-message-id

message-token is not guaranteed to be implemented by all CMs while
pending-message-id is (for incoming messages).
Comment 5 Guillaume Desmottes 2011-07-06 14:00:49 UTC
This needs https://bugs.freedesktop.org/show_bug.cgi?id=39000 which is merged but not yet released. I'll do a tp-glib release soon.
Comment 6 Emilio Pozuelo Monfort 2011-07-07 10:17:28 UTC
Comment on attachment 191399 [details] [review]
theme-adium: use the pending-message-id instead of message-token in x-empathy-message-id

Looks good, but also add a minimum tp-glib version bump when comitting this.
Comment 7 Guillaume Desmottes 2011-07-08 11:25:36 UTC
Attachment 191399 [details] pushed as c5efee1 - theme-adium: use the pending-message-id instead of message-token in x-empathy-message-id