GNOME.org
 

Re: [Evolution-hackers] Bug #127546 Gaim Evolution presence integration.


Wow, cool.

Ok there are some issues with the mail part of the patch, but they should be fixable fairly easily (well, 'fairly' is relative).

For starters you've put the formatter in the wrong place, it should really go only into the display formatter (emformathtmldisplay) - you don't really care for this information on printouts for example.

Secondly, you shouldn't be using g_object_set_data to set the EMIMPresence object onto the formatter - you need to just alllocate it directly in the EMFormatHTMLDisplay part.

The other problems are bigger.  And that is the way the presence info is being passed back and forth.  You're effevtively using a global variable to set it up, and then expecting it to format right.  This is just wrong, nothing else does this, so it shouldn't do it either.  Do not add a new uri type for it.

If you just want to insert an image, with no interactivity (which imho you don't want to do), you need to allocate a 'PURI' (pending uri), and then insert the image using the pending uri "cid" (content id) as the source.  You'll get a callback when the uri is requested, and you can do whatever you want there.  You really need to do this since the actual format_presence function *will* be called from another thread.  So what you have will likely lead to crashes (unless somehow your code only calls mt-safe apis).

You actually probably want to insert a custom widget anyway, in which case you have to use a PObject (pending object), and that works similarly.  That way you can do something if the user clicks on it.  It also lets you do incremental/asynchronous processing easier since you have more control over it.

You really also need to make sure all of the backend processing, like looking up presence and so forth, is asynchronous.  You can't block the whole mail display whilst you're looking up the presence information.  The api's you're using look very synchronous.

The EMIMPresence stuff shouldn't be looking inside the emfolderview and listening to message list events AT ALL.  It shouldn't need to know anything about totally unrelated widgets.  It should be totally self contained.  The api needs a bit of a re-think.  The formatter needs to drive it completely, including initiating a query and updating the display when the query returns, etc.  The formatter has access to the message - currently you're re-loading the message again for example.  It should also be a proper gobject, so the formatter can listen to it and re-draw if it needs to.

Actually the way I would do it is: create a new widget type, EMPresenceIcon.  In the formatter code in EMFormatHTMLDisplay), insert an <object> tag with the content-id of a newly-allocated HTMLPObject.  In the object callback, instantiate the EMPresenceIcon and pass it the email address as a a constructor argument.  This is all you do in the formatter.  The EMPresenceIcon fires off a query at this point, and does nothing else.  When the query comes back it updates itself.  If it doesn't/and when the widget is destroyed, it just cleans everything up.  Much simpler, fewer unrelated hooks which break abstraction layers (i.e. less spaghetti which is a maintenance nightmare), more efficient (no loading the message twice), and it gives you the option of making the icon a button you can do other cool things with.

You also have c99 stuff all over the place, you must define all of your variables in the declaration block before any statements occur (i.e. no in-line variable declarations).  Apart from a standard we just use for portability - it makes the code a lot easier to maintain going forward (some might argue about that, but believe me it makes a difference).

On Tue, 2004-09-07 at 03:17 -0400, praveen kandikuppa wrote: 
Hi,
            Please refer to the patch posted at
http://bugzilla.gnome.org/show_bug.cgi?id=127546. The patch implements
all the required features as mentioned on the webpage
http://www.gnome.org/bounties/IM.html#127546 . I am posting the patch
here as a few issues are left unresolved and i would like to get some
feedback on the patch.

The following issues are unresolved and i would really like some help
solving them.

No presence information is added to the contact editor. I was not sure
where exactly to add. I asked this particular quetion on the list but
nobody replied. Also the screen names can be edited in the contact
editor and hence the presence information may not always be in sync
and i felt that it is really not needed as the information is
automatically updated in the addressbook view as soon as a contact is
modified/added/removed.

I could not make the columns showing presence info in addressbook
table default . Right now you have to right click on the table and add
the column "IM Presence manually".

The floating dialog which appears when you click on presence icons is
closed as soon as mouse leaves the dialog box. Ideally it should stay
on as long as the mouse is clicked outside the dialog box.
            Any pointers on solving the above two issues are
appreciated. Regarding the contact-editor issue can somebody tell me
where exactly to put the icons/text indicating presence.
 
Other than what is mentioned above everything mentioned on the webpage
is implemented. The patches for evolution are split as i could not run
the diff properly for the whole evolution directory.

The summary of the changes are --
 
gaim-remote plugin is now multithreaded so as not to block the gaim UI.
there is a separate library named libevogaimremote.so which can be
used to issue presence queries. the result is conveyed back using
callbacks. The library current has API applicable only to get back
buddy information and send gaim uri requests which were required for
evolution. I will probably implement all the request types as
mentioned in remote.h later.

EText is modified to put clickable icons at arbitrary places inside
the EText widget. It is used right now to put icons in TO:,CC:,BCC:
fields in the compoer and also for the Minicard view.

HTML display of messages is modified to include clickable buddy icons
indicating  the contact the message is from.

Addressbook list view and minicard view are modified to include
presence information.

TO, BCC, CC fields are modified to include presence icons.

Select names table is modified to include presence icons. 

How to apply patches.
               there should be 8 patch files.
                                evolution-addressbook.diff,
evolution-art.diff, evolution-configure.diff, evolution-e-util.diff,
evolutio-mail.diff, evolution-widgets-misc.diff, gal.diff, gaim.diff.
The patches can be applied from within the specific application
directories with "patch -p1 < xxx.diff". For evolution and gal the CVS
versions can be used. I tested them with CVS versions from yesterday.
For gaim please use the latest stable release gaim-0.82.1. The gaim
from CVS has changed quite a lot and i am not even able to compile
gaim cvs version on my computer. So the patch is for the latest stable
release. I will modify it when the gaim CVS becomes stable/when i can
compile it. Regarding icons, 4 of them are needed. buddy-online.png,
buddy-offline.png, buddy-away.png and buddy-none.png. The first three
can be gotten from the page which describes the bounty. The last one
is used for a contact who is online/offline but has no contact
photo/buddy icon associated. Please use any icon for that right now.
All the above four icons must be present in art subdirectory of
evolution.
 
                            Please test/verify the patch and post back
if there are any problems. Also could somebody tell me if the patch
can be posted to evolution-patches list.
_______________________________________________
evolution-hackers maillist  -  evolution-hackers lists ximian com
http://lists.ximian.com/mailman/listinfo/evolution-hackers
--
Michael Zucchi <notzed ximian com>
"born to die, live to work, it's all downhill from here"
Novell's Evolution and Free Software Developer
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]