Opened 12 years ago

Closed 10 years ago

Last modified 10 years ago

#220 closed patch (fixed)

Pidgin does not display pictures recieved from Gadu Gadu

Reported by: MarkS Owned by: bartosz
Milestone: 2.5.3 Component: Gadu-Gadu
Version: 2.0 Keywords:
Cc: datallah

Description (last modified by evands)

Adium ticket: 4702

GG lets you send a picture (max size between 20 KB and 255 KB depending on the receiver's preferences) to a user. It is displayed in the chat window. However, Pidgin does not display the picture.

Attachments (10)

gg_image_support+basic_formatting.file (15.3 KB) - added by evands 11 years ago.
patch from #1575266: "Gadu Gadu images + txt format"
libpurple_gg_image_support.patch (16.4 KB) - added by nosnilmot 11 years ago.
Patch updated for libpurple
gg-images.tar.gz (3.8 KB) - added by sabby7890 11 years ago.
GG Image patches (diff -u)
gg-patch.2.diff (12.0 KB) - added by sabby7890 11 years ago.
Updated (uses GList)
gg-patch.3.diff (12.9 KB) - added by sabby7890 11 years ago.
Fixed receiving multiple images
gg-patch.diff (14.1 KB) - added by sabby7890 11 years ago.
Fixed bug that caused emoticon only messages (like <dresik>) to disappear
gg-patch.4.diff (14.1 KB) - added by sabby7890 11 years ago.
Fixed patch that made Pidgin crash sometimes when speaking with person that was using Tlen messenger
libpurple_gg_image_support.2.patch (13.5 KB) - added by ono 11 years ago.
Fixed sabby gg-patch v4
libpurple_gg_image_support.3.patch (29.4 KB) - added by ono 11 years ago.
Rearanged and revisited image support patch, should be fine now.
libpurple_gg_image_support.4.patch (14.2 KB) - added by ono 11 years ago.
Clean patch, only image support (w/o purple_debug comment fixes)

Download all attachments as: .zip

Change History (59)

comment:1 Changed 12 years ago by lschiere

  • Owner set to bartosz

comment:2 Changed 12 years ago by MiKom

I'll just add that sending images is implemented in libgadu.

comment:3 Changed 11 years ago by seanegan

  • Component changed from libpurple to Gadu-Gadu

comment:4 Changed 11 years ago by evands

From sf.net patch #1575266, here is the patch from biedron submitted 2006-10-11 which he wrote fixed this.

Changed 11 years ago by evands

patch from #1575266: "Gadu Gadu images + txt format"

comment:5 Changed 11 years ago by MarkS

This patch has not been committed in over a year(2006-10-11). Just curious if you found a reason this patch hasn't been committed. No presure but it would be great to hear some feedback. Thanks.

comment:6 Changed 11 years ago by Sim-on

  • Type changed from enhancement to patch

comment:7 Changed 11 years ago by nosnilmot

This patch needs porting from Gaim -> libpurple, and updating for the changes in the imgstore API.

I have done a quick update of it but have no way to test this (it compiles, although with more warnings than I would like).

Changed 11 years ago by nosnilmot

Patch updated for libpurple

comment:8 Changed 11 years ago by nosnilmot

I just noticed this leaks, which will need fixing if it is applied:

msg = g_string_new(g_markup_escape_text(tmp, -1));

comment:9 follow-up: Changed 11 years ago by liviopl

I wonder if libpurple could simply use features from libgadu.

comment:10 in reply to: ↑ 9 Changed 11 years ago by MarkS

Replying to liviopl:

I wonder if libpurple could simply use features from libgadu.

I was wondering the same thing. http://toxygen.net/libgadu/releases/1.8.0.html

comment:11 Changed 11 years ago by liviopl

And there are such nice news!

  • file transfer with Gadu-Gadu 7,
  • authorization with SHA-1,
  • support for XML-based info sent by server...

Pidgin treats all protocols as orphans (treats them unfair).

comment:12 Changed 11 years ago by evands

Building with the libgadu 1.8.0 files works for me without warnings, but I have no ability (or time) to test this configuration thoroughly. Please try the same and let us know what your results are.

comment:13 Changed 11 years ago by AlexJ

Gadu-Gadu attends images with max size of 255kB, not 20kB ;). 20kB is the smallest-size. The max img size is set-up by the Gadu-Gadu user.

comment:14 Changed 11 years ago by sabby7890

Ok, made some changes. Gadu-Gadu right now IS displaying the picture received from Gadu-Gadu (with formatting, too, so someone can send you pic like this:

message(pic)messagemessage(pic).

Just download my gg.c and gg.h files I've put here and receiving should work flawlessly. I am also working on sending images - works as for now (with original Gadu-Gadu client, just remember to be online), but without formatting and with <IMG ID="x"> message. Stay tuned, don't have time to create diffs, so just download gg.c and gg.h files, put them in your pidgin-2.4.1/libpurple/protocols/gg/ directory and it should work.

If someone wants to have the latest version of my work, go here:

http://dp0154.debowypark.waw.pl/gg.c
http://dp0154.debowypark.waw.pl/gg.h

Cheers!

comment:15 Changed 11 years ago by ono

@sabby7890: Please put your patch as single file as described in TipsForPatchSubmissions

Regarding your latest patch:

  1. msg = charset_convert(msg, "UTF-8", "CP1250"); and some other g_strdup_printf in ggp_send_im_richtext are IMHO memory leaks, since you don't free anywhere old msg memory buffer.
  1. UTF-8 -> CP1250 should happen at the very end just before sending the buffer to libgadu function, as all glib functions such as g_strsplit work in UTF-8 locale, so doing conversion to CP1250 first then working with UTF-8 aware functions if pretty bad idea since it may lead to unpredictable behavior.

Anyway I really appreciate your work and looking towards putting it into repository once all the memory related things are sorted out.

comment:16 Changed 11 years ago by sabby7890

@ono

Sorry for long time to reply, I'm getting to work right now:) Thanks for your tips, everything will be fixed shortly (I believe today)

comment:17 Changed 11 years ago by AlexJ

Would anybody change the Ticket's description, where is written "max size 20KB" to the true one? In original client, user can specify how much big images he wants to receive (he can choose between 20 KiB - 255 KiB), and if somebody sends him too big image, the sender's client shows message about it and probably the reveiver also get know about it. I also have a proffer about sending a part of screenshot (made automatically) to the reveiver which is a very useful function (didn't it?). I hope that images support in Gadu-Gadu protocol will be implemented in the near future, maybe next stable version of Pidgin (it's very annoying when somebody sends you a picture, but you only see an empty message, or he tells you that you should change the max img size in the Gadu-Gadu client settings to 255 KiB). Thanks, and cheers!

comment:18 Changed 11 years ago by evands

  • Description modified (diff)

comment:19 Changed 11 years ago by sabby7890

Ok, I'm still working on it, so if anyone is using my patch right now please, contact with me on Gadu-Gadu: 3700457 or Google Talk: tsalacinski@… to report bugs before I'll release it. I need to do a couple of things to be sure this patch will be rock stable.

comment:20 Changed 11 years ago by sabby7890

Ok, done some work, right now we should be freeing everything (I think?).

Please, check this out. Also - the patches are in unified (diff -u) format.

If you have any suggestions, please feel free to IM me. Sorry it takes so long, I have some other things to do that need to be done first:( Cheers!

Fixed: few memory leaks and character conversion, also when someone is sending us an image there is no null message (for ex. when someone sends an image to us, first there is a message without anything, then it shows the valid message).

Changed 11 years ago by sabby7890

GG Image patches (diff -u)

comment:21 Changed 11 years ago by sabby7890

Ok, uploaded new patch file. Should be fine - I hope there are no memory leaks, image support should be better (but from dev point of view - I know, far from perfect;) ).

Changed 11 years ago by sabby7890

Updated (uses GList)

Changed 11 years ago by sabby7890

Fixed receiving multiple images

comment:22 follow-up: Changed 11 years ago by tpg

Looks like the patch is working like a charm ;)

comment:23 in reply to: ↑ 22 Changed 11 years ago by sabby7890

Replying to tpg:

Looks like the patch is working like a charm ;)

Glad you like it. I'm still working on it, so try the new patch (preview of text formatting, receiving only - bold, italic, underline).

Any help appreciated.

Changed 11 years ago by sabby7890

Fixed bug that caused emoticon only messages (like <dresik>) to disappear

comment:24 Changed 11 years ago by MarkS

sabby7890:

Is your latest patch (gg-patch.dff) feature complete and you feel the patch can be committed or do you feel it still needs more work.

By the way, good work and thanks for the hard work.

comment:25 Changed 11 years ago by sabby7890

MarkS:

It is not feature complete (it doesn't support images in conference windows), but I think it can be committed.

comment:26 Changed 11 years ago by sabby7890

I think this cannot be pushed yet. Messages in Gadu-Gadu sometimes crash Pidgin (it happened twice to me), not very often, but they do. I have to work on it.

Changed 11 years ago by sabby7890

Fixed patch that made Pidgin crash sometimes when speaking with person that was using Tlen messenger

comment:27 Changed 11 years ago by sabby7890

I think it can be committed right now. I haven't noticed any crash or bad work when I was using this patch.

comment:28 Changed 11 years ago by ono

+1

I think this patch is now alright, thanks for your hard work and testing Sabby.

comment:29 Changed 11 years ago by ono

Please DON'T commit it yet. It has few bugs inside, sorry I just revised this patch.

Changed 11 years ago by ono

Fixed sabby gg-patch v4

comment:30 Changed 11 years ago by ono

Just to not be "annoying teacher" I'm attaching fixes to sabby patch above. This needs to be tested though:

Fixes are:

  • proper formatting and C / Doxy compatible comments
  • pending_richtext_messages are now part of GG proto instance data
  • pending_images hash instead of looking trough all images for valid image with crc to send
  • few fixes, about not released string buffers.. or unnecessary variables, etc. just to make code smaller and cleaner

It is getting late, tommorow I'll be testing this patch hopefully. Waiting for your comments.

comment:31 Changed 11 years ago by sabby7890

Ono, I've started to write a message to ask you for exactly the same thing you just did, then it showed me that someone else modified the bug:) Believe me, for me anyone can thrash my patch, tell me it's very bad etc, but if someone points me to the right direction/fixes my patch, it's VERY appreciated. I won't take a look at it today, though, it's late here too. Goodnight:)

Changed 11 years ago by ono

Rearanged and revisited image support patch, should be fine now.

comment:32 Changed 11 years ago by ono

Okay I think the patch I just attached libpurple_gg_image_support.3.patch (reworked Sabby's) patch is good enough. Please check it, I've checked it with latest 7.7 client. @Sabby please install this patch on fresh mtn gg.c & gg.h releases, and try to test it. It should be safe enough to commit it into the trunk.

In comparison to previous patch:

  • fixed UTF-8 positioning in ggp_recv_message_handler, so result string is valid UTF8 after injecting HTML tags
  • fix incoming bold, italic, etc. formatting, GG is sending empty bogus formatting packets, that shouldn't be treated as formatting end
  • richtext sending is now embedded in ggp_send_im no need for extra function, also safer return values
  • purple_markup_find_tag used for detecting img tags -> safer than g_split
  • fixed all purple_debug to include function names.. so they are more descriptive

Changed 11 years ago by ono

Clean patch, only image support (w/o purple_debug comment fixes)

comment:33 Changed 11 years ago by MarkS

Just curious...Will the changes in the patch also fix the formatting issue that were reported in Ticket 222?

comment:34 Changed 11 years ago by sabby7890

I don't know how to reproduce it, never had any problems with links in Pidgin.

comment:35 Changed 11 years ago by MarkS

It was a more of a formating issue. It was related to this Adium ticket 6151. It explains the issue more clearly.

comment:36 Changed 11 years ago by ono

Well right now, this patch just adds support for richtext message reception (bold, italic, underline + inline messages), while no richtext message sending but just image sending trough <img id="ID"> mapping.

Obviously we need to sort Adium out somehow, since it removes <img> tags too, so actually image sending works in pidgin while not in Adium AFAIK.

I'll try expend this patch to add <b>, <i>, <u> mapping, too, then I need to remove rest of html mapping. Probably this removal should happen directly in the plugin, right?

Then I'll try to add propriate patch to Adium too. So we got more complete GG support. Cheers!

comment:37 Changed 11 years ago by liviopl

So we got more complete GG support.

Remember about "more". Audio (and video?) chats left, for example.

comment:38 Changed 11 years ago by sabby7890

I think instead of just images this patch should contain complete richtext support (ie. colored text). I'll try to work on it, too.

comment:39 Changed 11 years ago by Sim-on

hey guys, this image stuff would be really nice but at the moment the latest gadu-gadu version doesnt work at all... (#6232)

comment:40 Changed 11 years ago by MarkS

ono:

Adium is based on libpurple so if it is fixed here once committed it should get automatically fixed in the Adium once Evan pushes the latest libpurple. This is why I hang out here to see what progress has been made if any. Try to test the new gadu libpurple features in Adium if there are any changes. I considered rich text a nice to have, but it was not considered an essential feature. However, having it would bring Pidgin one step closer to actually client. By the way, I want to thank you as well as sabby7890 for improving the Gadu Gadu experience on Pidgin/Aduim?. Job well done.

comment:41 Changed 10 years ago by MarkS

  • Milestone set to Patches Needing Improvement

comment:42 Changed 10 years ago by sabby7890

MarkS:

Please tell me, what is needed to be done in order to improve this patch and commit it to Pidgin? AFAIK last time I used it it worked fine.

comment:43 Changed 10 years ago by MarkS

I thought you were still working on the patch per this comment.

"Replying to sabby7890:

I think instead of just images this patch should contain complete richtext support (ie. colored text). I'll try to work on it, too."

If you feel this patch works fine, let see if on of the Pidgin developers can get this patch committed. Can any Pidgin developer on this distro list review sabby7890's latest revision of the patch?

Thanks for the hard work.

I have seen a quite a few patches for gadu that have been just sitting idle However, I would like to see yours committed.

=======================

Here are some example of patches for gadu that have been collecting dust. (Ex. 373, 4582, 5175, 6270, 6691)

comment:44 Changed 10 years ago by sabby7890

I see these patches, and IMHO lot of them provide great functionality to Pidgin. They cannot be applied at once, though. First, upgrading libgadu is necessary (bug 373). Then we can apply rest of the patches. AFAIK my patch is working fine, I had some trouble with it on x64 machine. I'll try to merge these patches to one big patch and send it to pidgin devs. It will take some time though, I don't have much time (developing Alarm Clock - alarm-clock.54.pl) and I just wanted to have some personal life:) When I'll find some time, I'll just post a new bug with big patch.

comment:45 Changed 10 years ago by ono@…

  • Milestone changed from Patches Needing Improvement to 2.5.3
  • Resolution set to fixed
  • Status changed from new to closed

(In bc507450c4a8e842eb0ab445428935652307df88):
Support displaying buddy icons from Gadu-Gadu buddies. Fixes #220.

comment:46 Changed 10 years ago by MarkS

Glad to see this committed but shouldn't the description say "Fixes sending/receiving a picture or image via a chat in Gadu Gadu" instead of "Support displaying buddy icons from Gadu-Gadu buddies." Since support for displaying icon was addressed in this ticket 4971 .

comment:47 Changed 10 years ago by rekkanoryo@…

(In 57f0a80156490200a80b4787d0be27ded56040c7):
I forgot to ChangeLog this earlier. Refs #220.

comment:48 Changed 10 years ago by nosnilmot@…

(In c156244970ba750784155b6bae31df621c20af02):
I believe the patch from #220 actually implemented IM images for Gadu-Gadu, not something to do with Buddy Icons. References #220

comment:49 Changed 10 years ago by rekkanoryo@…

(In df6eba32e5b6b34d7483cbfb7e9f2e4c836ac35f):
Correctly credit Tomasz Sałaciński for writing the original patch for IM images on Gadu-Gadu. I previously missed that the patch I applied was Tomasz's patch with modifications. Refs #220.

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!