Opened 11 years ago

Closed 11 years ago

#5627 closed patch (fixed)

Implement custom emoticons on XMPP using XEP 0231

Reported by: malu Owned by: sadrul
Milestone: 2.5.2 Component: XMPP
Version: 2.4.1 Keywords: xmpp custom emoticons
Cc:

Description

This ticket is about adding support for sending and receiving custom emoticons on XMPP using XEP 0231 ("Data Element") and the use-cases 3.1 (messaging) and 3.2 (data retreival) define therein, using the libpurple smiley API as implemented by salinas_v and msfbrasil.

I have starting on a patch implementing this on branch im.pidgin.pidgin.custom_smiley.

The XEP also specifies other use-cases (for example for providing an image thumbnail in a file transfer request), but these will not be covered in this ticket...

Attachments (19)

xmpp-custom-emoticons.diff (31.7 KB) - added by malu 11 years ago.
Implements custom emoticons for XMPP
xmpp-custom-emoticons2.diff (32.5 KB) - added by malu 11 years ago.
Fixed freeing emoticons when closing a MUC
xmpp-custom-emoticons3.diff (32.5 KB) - added by malu 11 years ago.
Fixed memory leak in data.c
xmpp-custom-emoticons4.diff (32.8 KB) - added by malu 11 years ago.
Set flag PURPLE_CONNECTION_ALLOW_CUSTOM_SMILEY on PurpleConnection? for XMPP
philled.png (20.1 KB) - added by phannent 11 years ago.
Screenshot of failed receiving of the custom emoticon, see an empty line
xmpp-custom-emoticons4.1-by-phil.diff (33.3 KB) - added by phannent 11 years ago.
Updated for Win32 compile
xmpp-custom-emoticons5.diff (37.7 KB) - added by malu 11 years ago.
Added support for service discovery of "in-band images".
xmpp-fix-win32-build.diff (1.3 KB) - added by phannent 11 years ago.
Fix for building on Windows
xmpp-custom-emoticons-data-leakfix.diff (2.8 KB) - added by malu 11 years ago.
Use destructor functions for the per-conversation-allocated hash tables (fixes freeing keys)
xmpp-custom-emoticons-refactor.diff (13.8 KB) - added by malu 11 years ago.
Some changes in message.c to simplify fetching unique smileys from incoming messages. data.c now uses purple_conversation_get/set to store data hashtables per conversation
xmpp-custom-emoticons-fix-hardcoded-namespace.diff (14.5 KB) - added by malu 11 years ago.
Changed one occurance of a hard-coded XML namespace in iq.c to use #define:d string
xmpp-custom-emoticons-20080610.diff (15.6 KB) - added by malu 11 years ago.
Handle <img>...</img> and lack of "alt" attribute
xmpp-custom-emoticons-6-by-phil.diff (16.3 KB) - added by phannent 11 years ago.
mtn diff with all patches since 1st June
xmpp-custom-emoticons-20080713.diff (18.1 KB) - added by malu 11 years ago.
Added function in libpurple/util.c to return the hostname
xmpp-custom-emoticons-20080831.diff (30.8 KB) - added by malu 11 years ago.
Updated to latest XEP
xmpp-custom-emoticons-20080903.diff (30.8 KB) - added by malu 11 years ago.
Changed namespace according to change to draft
compile.log (2.5 KB) - added by phannent 11 years ago.
Test compile of patch: xmpp-custom-emoticons-20080903.diff
util.c.rej (591 bytes) - added by phannent 11 years ago.
Rejected from patch 20080903
xmpp-win-fix.patch (573 bytes) - added by phannent 11 years ago.
Fix for the window compile

Download all attachments as: .zip

Change History (55)

comment:1 Changed 11 years ago by salinasv

suscribe

Changed 11 years ago by malu

Implements custom emoticons for XMPP

comment:2 Changed 11 years ago by malu

Here is a first patch against im.pidgin.pidgin.custom_smiley implementing custom emoticons using XEP 0231. It works quite nice for me. I haven't tried it much with MUCs. There also seems to be an issue when sending messages to offline buddies, where sometimes the message is silently dropped (but not always) when containing an emoticon. This may be because of a size limitation.

comment:3 Changed 11 years ago by sadrul

  • Owner changed from nwalp to sadrul

comment:4 Changed 11 years ago by sadrul

  • Owner changed from sadrul to deryni

Actually, I think deryni would be the better person to look at this. I am not very familiar with our jabber code.

Changed 11 years ago by malu

Fixed freeing emoticons when closing a MUC

comment:5 Changed 11 years ago by malu

I just realized "jabber_convo_closed" isn't called when closing a MUC. So, here is an updated patch that calls "jabber_data_delete_associated_with_conv" for the PurpleConversation? belonging to a JabberChat? in "jabber_chat_part".

Changed 11 years ago by malu

Fixed memory leak in data.c

Changed 11 years ago by malu

Set flag PURPLE_CONNECTION_ALLOW_CUSTOM_SMILEY on PurpleConnection? for XMPP

comment:6 Changed 11 years ago by malu

The last patch sets the new PURPLE_CONNECTION_ALLOW_CUSTOM_SMILEY flag on the PurpleConnection? when logging in on an XMPP connection. This allows custom emoticons to be selected on XMPP.

comment:7 follow-up: Changed 11 years ago by phannent

Hello,

I just patched this to the next.minor branch and compiled on windows. Due to Glib being version 2.0 rather than 2.8, I had to change line 62 to this:

	char hostname[256];
	int ret;

	ret = gethostname(hostname, sizeof(hostname));
	hostname[sizeof(hostname) - 1] = '\0';
	if (ret < 0 || hostname[0] == '\0') {
		purple_debug_warning("irc", "gethostname() failed -- is your hostname set?");
		strcpy(hostname, "localhost");
	}
	
	/* is there some better way to generate a content ID? */
	g_snprintf(cid, sizeof(cid), "%s@%s", checksum, hostname);

plus it failed to receive at the other end (see that screenshot I am going to attach).

If anybody wants to try it on windows its here: http://www.hannent.eu/pidgin/20080513/xmpp/pidgin-2.5.0devel.exe

Changed 11 years ago by phannent

Screenshot of failed receiving of the custom emoticon, see an empty line

comment:8 in reply to: ↑ 7 Changed 11 years ago by malu

Replying to phannent:

Hello,

I just patched this to the next.minor branch and compiled on windows. Due to Glib being version 2.0 rather than 2.8, I had to change line 62 to this:

	char hostname[256];
	int ret;

	ret = gethostname(hostname, sizeof(hostname));
	hostname[sizeof(hostname) - 1] = '\0';
	if (ret < 0 || hostname[0] == '\0') {
		purple_debug_warning("irc", "gethostname() failed -- is your hostname set?");
		strcpy(hostname, "localhost");
	}
	
	/* is there some better way to generate a content ID? */
	g_snprintf(cid, sizeof(cid), "%s@%s", checksum, hostname);

plus it failed to receive at the other end (see that screenshot I am going to attach).

If anybody wants to try it on windows its here: http://www.hannent.eu/pidgin/20080513/xmpp/pidgin-2.5.0devel.exe

I will take a look at your patch and integrate it with my patch. Maybe a gethostname function should be in libpurple?

Does the empty line happen when both ends have the patch? The current Pidgin doesn't handle <img> tags in XHTML-IM. The smileys get translated to such tags when sending the message. They are "unpacked" by the receiver (and the text is "smileyfied" using the functions available in libpurple. (if custom smileys have been turned off the tag will just be converted to the corresponding shortcut text (the "alt" attribute).

comment:9 Changed 11 years ago by phannent

You were right with both sides needing the patch.

Changed 11 years ago by phannent

Updated for Win32 compile

comment:11 Changed 11 years ago by ferk

wow.. this is really great, thank you so much! is there any built .deb already for test?

comment:12 Changed 11 years ago by ferk

Oh.. sorry.. echelon89 already did it.. thank you! here it is echelon89 page: [ http://echelon89.altervista.org/download/patchato/view-category.html?dir=DESC&limit=5&limitstart=0&order=date click]

comment:13 follow-ups: Changed 11 years ago by sadrul

mlundblad: I believe it's important that if the client doesn't support custom smileys (e.g. an unpatched pidgin) then it will display the text for the received custom smiley, instead of completely removing it (which appears to be the current state, judging from the screenshot).

comment:14 in reply to: ↑ 13 Changed 11 years ago by malu

Replying to sadrul:

mlundblad: I believe it's important that if the client doesn't support custom smileys (e.g. an unpatched pidgin) then it will display the text for the received custom smiley, instead of completely removing it (which appears to be the current state, judging from the screenshot).

Unfortunatly the current (unpatched) GtkIMHtml ignores <img> tags. So I haven't come up with a good way to let old clients to display just the shortcut. The best would have been if there was a defined capability that the client was supposed to present in case it supports images with embedded data. The plain-text body element of the message stanza on the other hand contains just the text of the smileys, so clients not knowing XHTML-IM extension will display the raw text.

comment:15 in reply to: ↑ 13 Changed 11 years ago by malu

Replying to sadrul:

mlundblad: I believe it's important that if the client doesn't support custom smileys (e.g. an unpatched pidgin) then it will display the text for the received custom smiley, instead of completely removing it (which appears to be the current state, judging from the screenshot).

Now we have a service discovery for the messageing use case of XEP-0231. So it shall be able to just insert the text for a smiley if the other client doesn't advertize support. I've starting working on a new patch adding that functionallity.

Changed 11 years ago by malu

Added support for service discovery of "in-band images".

comment:16 Changed 11 years ago by malu

The latest patch adds checking for the capability of receiving smileys "in-band images" using the updated XEP-0231. Currently it doesn't attempt to determine support when in an MUC, so smileys will be sent as plain text there.

comment:17 Changed 11 years ago by sadrul

  • Type changed from enhancement to patch

comment:18 Changed 11 years ago by ml@…

(In 144341017ede7e3be12c467ddec33984a0868928):
Custom smileys for XMPP according to XEP 0231. Refs #5627.

Changed 11 years ago by phannent

Fix for building on Windows

comment:19 Changed 11 years ago by phannent

Hello,

I recompiled the new im.pidgin.xmpp.custom_smiley branch with the attached fix and a windows binary can be found here: http://www.hannent.eu/pidgin/20080602/pidgin-2.5.0devel.exe

Changed 11 years ago by malu

Use destructor functions for the per-conversation-allocated hash tables (fixes freeing keys)

comment:20 Changed 11 years ago by malu

This last patch uses destructor functions on "inner hash tables" used to cache smileys. Fixes a few leaks...

Changed 11 years ago by malu

Some changes in message.c to simplify fetching unique smileys from incoming messages. data.c now uses purple_conversation_get/set to store data hashtables per conversation

comment:21 Changed 11 years ago by malu

I have uploaded a new patch. There is some modification in the function that parses the XHTML of incoming messages to get a list of unique smileys. Also the cacheing of JabberData? objects per conversation in jabber/data.c now uses the functions for PurpleConversation? to store data attributes to store the hash tables for local and remote data items per conversation.

Changed 11 years ago by malu

Changed one occurance of a hard-coded XML namespace in iq.c to use #define:d string

Changed 11 years ago by malu

Handle <img>...</img> and lack of "alt" attribute

comment:22 Changed 11 years ago by malu

Updated patch:

This one can cope with a message where <img> tags are not self-closing, ie. <img alt=".." src="..."></img>

It also handles the case when the received <img/> doesn't have an "alt" attribute, in this case the remote shortcut of the smiley is set to the value of the content ID (cid), since the smiley API needs a shortcut.

Changed 11 years ago by phannent

mtn diff with all patches since 1st June

comment:23 Changed 11 years ago by phannent

I have also built the windows binary which can be downloaded from: http://hannent.blogspot.com/2008/06/emoticon-xmpp-pidgin.html

Changed 11 years ago by malu

Added function in libpurple/util.c to return the hostname

comment:24 Changed 11 years ago by malu

The last patch add a function in util.c to return the host name, it uses g_get_host_name on glib >= 2.8 and usis Phil's code if not.

comment:25 Changed 11 years ago by sadrul@…

(In ee0b2e3380c2604a456e2388ce05e8ba8913f4f8):
Add purple_get_host_name to get the hostname of the machine. This is a replacement for g_get_host_name in glib2.8+. Thanks to Phil Hannent and Marcus Lundblad for the initial patch. References #5627.

comment:26 in reply to: ↑ description Changed 11 years ago by scytheman

cc

Changed 11 years ago by malu

Updated to latest XEP

comment:27 Changed 11 years ago by malu

The latest patch updates the implementation according to the latest specification. Received data objects are cached per CID instead of per conversation. CIDs is generated of the form "sha1+hash@…". Only data objects of size < 1024 bytes are included directly in the <message/> stanza. (data is requested using <iq/>s if not already cached by the receiver).

Changed 11 years ago by malu

Changed namespace according to change to draft

comment:28 Changed 11 years ago by malu

The last patch changes the namespace of the <data/> element (and capabilities), since the XEP is now draft, and has dropped ":tmp"

Changed 11 years ago by phannent

Test compile of patch: xmpp-custom-emoticons-20080903.diff

Changed 11 years ago by phannent

Rejected from patch 20080903

comment:29 Changed 11 years ago by phannent

I have attached the compile log from the 20080903 patch as it did not compile cleanly on windows.

This was against a clean im.pidgin.xmpp.custom_smiley which badly needs a propogate from i.p.p

comment:30 Changed 11 years ago by phannent

Scratch my last comment, I just saw that I had propagated from i.p.p to this branch and that is what is causing the error.

comment:31 Changed 11 years ago by sadrul

  • Owner changed from deryni to sadrul

Changed 11 years ago by phannent

Fix for the window compile

comment:32 Changed 11 years ago by phannent

Sadrul shepherded me through the fix for the windows build. Malu could you apply the attached patch to the branch please?

comment:33 Changed 11 years ago by phannent

comment:34 Changed 11 years ago by phannent

The recent patches all applied fine and are working great, windows builds are here: http://hannent.blogspot.com/2008/09/custom-emoticons-working-with-jabbim.html

comment:35 Changed 11 years ago by phannent

Can this be merged? I have no problems with running this and want to get on with testing IBB #6183

comment:36 Changed 11 years ago by malu@…

  • Milestone set to 2.5.2
  • Resolution set to fixed
  • Status changed from new to closed

(In d5543e6ef5e67e9c4a1fb372c06245ab36155e4b):
Add custom smiley support for XMPP. Closes #5627.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!