Re: [evolution-patches] Patch for the receipt bounty (#127534)
- From: Not Zed <notzed ximian com>
- To: ERDI Gergo <cactus cactus rulez org>
- Cc: Jeffrey Stedfast <fejj ximian com>, asdf <evolution-patches lists ximian com>
- Subject: Re: [evolution-patches] Patch for the receipt bounty (#127534)
- Date: Fri, 20 Aug 2004 20:54:48 +0800
Most of the composer stuff looks good.
> EDestination **
> Index: composer/e-msg-composer.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/composer/e-msg-composer.h,v
> retrieving revision 1.91
> diff -u -u -r1.91 e-msg-composer.h
> --- composer/e-msg-composer.h 27 Jul 2004 16:52:17 -0000 1.91
> +++ composer/e-msg-composer.h 16 Aug 2004 12:10:59 -0000
> @@ -101,6 +101,7 @@
> guint32 view_bcc : 1;
> guint32 view_cc : 1;
> guint32 view_subject : 1;
> + guint32 request_receipt : 1;
> guint32 has_changed : 1;
> guint32 autosaved : 1;
As a general rule, add new options to the end of existing ones. There
are a lot of reasons for this, but it isn't a major issue.
> +2004-08-07 ERDI Gergo <
cactus cactus rulez org>
> +
> + * e-account.h: Added new receipt_policy field to services
> +
I'm not sure we actually want to add it to a service field. Its
really an account-relative option, nothing to do with the backend
you're using on that account.
On reflection, it should probably go in the core account info, and it
should go on the "Defaults" configuration page.
> 2004-08-05 Rodrigo Moya <
rodrigo novell com>
>
> * e-icon-factory.c (e_icon_factory_init): connect to "changed"
> Index: e-util/e-account.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/e-util/e-account.c,v
> retrieving revision 1.10
> diff -u -u -r1.10 e-account.c
> --- e-util/e-account.c 9 Apr 2004 19:47:06 -0000 1.10
> +++ e-util/e-account.c 16 Aug 2004 12:11:03 -0000
> @@ -244,6 +244,56 @@
> }
> }
>
> +static EAccountReceiptPolicy
> +str_to_receipt_policy (const char *str)
> +{
> + if (!strcmp (str, "ask"))
> + return E_ACCOUNT_RECEIPT_ASK;
> + if (!strcmp (str, "always"))
> + return E_ACCOUNT_RECEIPT_ALWAYS;
> +
> + return E_ACCOUNT_RECEIPT_NEVER;
> +}
> +
> +static char*
> +receipt_policy_to_str (EAccountReceiptPolicy val)
> +{
> + char *ret = 0;
> +
> + switch (val) {
> + case E_ACCOUNT_RECEIPT_NEVER:
> + ret = "never";
> + break;
> + case E_ACCOUNT_RECEIPT_ASK:
> + ret = "ask";
> + break;
> + case E_ACCOUNT_RECEIPT_ALWAYS:
> + ret = "always";
> + break;
> + }
> +
> + return ret;
> +}
I'd have used a table, but i guess this'll do. I've got some helper
stuff in the eplugin class to do this anyway.
> @@ -314,6 +364,7 @@
>
> changed |= xml_set_bool (node, "auto-check", &service->auto_check);
> changed |= xml_set_int (node, "auto-check-timeout", &service->auto_check_time);
> + changed |= xml_set_receipt_policy (node, "receipt-policy", &service->receipt_policy);
> if (service->auto_check && service->auto_check_time <= 0) {
> service->auto_check = FALSE;
> service->auto_check_time = 0;
> @@ -447,6 +498,7 @@
> dest->source->auto_check = src->source->auto_check;
> dest->source->auto_check_time = src->source->auto_check_time;
> dest->source->save_passwd = src->source->save_passwd;
> + dest->source->receipt_policy = src->source->receipt_policy;
>
> g_free (dest->transport->url);
> dest->transport->url = "" (src->transport->url);
> @@ -527,7 +579,7 @@
> xmlSetProp (src, "keep-on-server", account->source->keep_on_server ? "true" : "false");
> xmlSetProp (src, "auto-check", account->source->auto_check ? "true" : "false");
> sprintf (buf, "%d", account->source->auto_check_time);
> - xmlSetProp (src, "auto-check-timeout", buf);
> + xmlSetProp (src, "receipt-policy", receipt_policy_to_str
(account->source->receipt_policy));
I don't think you wanted to remove the other setting.
> @@ -187,6 +191,7 @@
> if (emcs && emcs->folder) {
> /* set any replied flags etc */
> camel_folder_set_message_flags (emcs->folder, emcs->uid, emcs->flags, emcs->set);
> + camel_folder_set_message_user_flag (emcs->folder, emcs->uid, "receipt-handled", TRUE);
> camel_object_unref (emcs->folder);
> emcs->folder = NULL;
> g_free (emcs->uid);
> @@ -1101,6 +1106,159 @@
> g_return_if_fail (uid != NULL);
>
> mail_get_message (folder, uid, redirect_msg, NULL, mail_thread_new);
> +}
> +
> +
> +static gboolean
> +em_utils_ask_receipt (CamelFolder *folder, CamelMimeMessage *message)
> +{
> + /* Check the account's receipt policy settings, and pop up a
> + * dialog if the policy is ASK */
> +
> + EAccount *account = guess_account (message, folder);
> +
> + if (account->source->receipt_policy == E_ACCOUNT_RECEIPT_ASK) {
> + const char *receipt_address = camel_medium_get_header (CAMEL_MEDIUM (message), "Disposition-Notification-To");
> + const char *subject = camel_mime_message_get_subject (message);
> +
> + return (e_error_run (NULL, "mail:ask-receipt", receipt_address, subject) == GTK_RESPONSE_YES);
> + }
> +
> + return (account->source->receipt_policy == E_ACCOUNT_RECEIPT_ALWAYS);
> +}
> +
> +void
> +em_utils_handle_receipt (CamelFolder *folder, const char *uid)
> +{
> + CamelMimeMessage *message = camel_folder_get_message (folder, uid, NULL);
> +
> + g_return_if_fail (message);
> +
> + if (camel_folder_get_message_user_flag (folder, uid, "receipt-handled"))
> + return;
> +#if 0
> + camel_folder_set_message_user_flag (folder, uid, "receipt-handled", TRUE);
> +#endif
> +
> + if (!camel_medium_get_header (CAMEL_MEDIUM (message), "Disposition-Notification-To"))
> + return;
> +
> + if (em_utils_ask_receipt (folder, message))
> + em_utils_send_receipt (folder, message);
> +}
This is a lot of code, this can all be done in one if _expression_ as
part of em_utils_send_receipt where it checks !receipt_address.
i.e.
{
EAccount *account = guess_account(message, folder);
const char *receipt_address = ...;
const char *subjec = ...;
if (receipt_address == NULL
|| account->source->receipt_policy == E_ACCOUNT_RECEIPT_NEVER
|| (account->source->receipt_policy == E_ACCOUNT_RECEIPT_ASK
&& e_error_run(parent, "mail:ask-receipt", receipt_address, subject) != GTK_RESPONSE_YES)
return;
> +static void
> +em_utils_receipt_done_cb (CamelFolder *folder, CamelMimeMessage *msg, CamelMessageInfo *info,
> + int queued, const char *appended_uid, void *data)
> +{
> + camel_message_info_free (info);
Should do mail_send() here, make sure the queued mail is sent off.
I also spotted another leak, you should also unref msg as well, since
you allocated it in send_receipt.
> +}
> +
> +void
> +em_utils_send_receipt (CamelFolder *folder, CamelMimeMessage *message)
> +{
> + EAccount *account = guess_account (message, folder);
> +
> + CamelMimeMessage *receipt = camel_mime_message_new ();
> + CamelMultipart *body = camel_multipart_new ();
don't add a blank line above, and don't allocate anything until we've worked out we need to send one.
i.e. after the first return statement, otherwise you leak those two objects.
> + CamelMimePart *part;
> + CamelDataWrapper *receipt_text, *receipt_data;
> + CamelContentType *type;
> + CamelInternetAddress *addr;
> + CamelStream *stream;
> + CamelFolder *out_folder;
> + CamelMessageInfo *info;
> + const char *message_id = camel_medium_get_header (CAMEL_MEDIUM (message), "Message-ID");
> + const char *message_date = camel_medium_get_header (CAMEL_MEDIUM (message), "Date");
> + const char *message_subject = camel_mime_message_get_subject (message);
> + const char *receipt_address = camel_medium_get_header (CAMEL_MEDIUM (message), "Disposition-Notification-To");
> + char hostname[MAXHOSTNAMELEN + 1];
> + char *self_address, *receipt_subject;
> +
> + if (!receipt_address)
> + return;
> +
> + /* Collect information for the receipt */
> + if (!gethostname (hostname, MAXHOSTNAMELEN) || !hostname[0])
> + strncpy (hostname, "localhost.localdomain", MAXHOSTNAMELEN);
> + self_address = account->id->address;
> + if (!message_id)
> + message_id = "";
> + if (!message_date)
> + message_date ="";
> +
> + /* Create toplevel container */
> + camel_data_wrapper_set_mime_type (CAMEL_DATA_WRAPPER (body),
> + "multipart/report;"
> + "report-type=\"disposition-notification\"");
> + camel_multipart_set_boundary (body, NULL);
> +
> + /* Create textual receipt */
> + receipt_text = camel_data_wrapper_new ();
> + type = camel_content_type_new ("text", "plain");
> + camel_data_wrapper_set_mime_type_field (receipt_text, type);
> + camel_content_type_unref (type);
> + stream = camel_stream_mem_new ();
> + camel_stream_printf (stream,
> + "Your message to %s about \"%s\" "
> + "(dated %s) has been read by the user.",
> + self_address, message_subject, message_date);
We could check for a Languages header in the original and translate to
one of those. But that might be getting a bit out of scope.
> + camel_data_wrapper_construct_from_stream (receipt_text, stream);
> + camel_object_unref (stream);
> +
> + part = camel_mime_part_new ();
> + camel_medium_set_content_object (CAMEL_MEDIUM (part), receipt_text);
> + camel_object_unref (receipt_text);
> + camel_multipart_add_part (body, part);
> + camel_object_unref (part);
> +
> + /* Create the machine-readable receipt */
> + receipt_data = camel_data_wrapper_new ();
> + type = camel_content_type_new ("message", "disposition-notification");
> + camel_data_wrapper_set_mime_type_field (receipt_data, type);
> + camel_content_type_unref (type);
> + stream = camel_stream_mem_new ();
> + camel_stream_printf (stream,
> + "Reporting-UA: %s; %s\n"
> + "Final-Recipient: rfc822; %s\n"
> + "Original-Message-ID:%s\n"
> + "Disposition: manual-action/MDN-sent-manually; displayed",
> + hostname, "Evolution " VERSION SUB_VERSION " " VERSION_COMMENT,
> + self_address, message_id);
> + camel_data_wrapper_construct_from_stream (receipt_data, stream);
> + camel_object_unref (stream);
> +
> + part = camel_mime_part_new ();
> + camel_medium_set_content_object (CAMEL_MEDIUM (part), receipt_data);
> + camel_object_unref (receipt_data);
> + camel_multipart_add_part (body, part);
> + camel_object_unref (part);
> +
> + /* Finish creating the message */
> + camel_medium_set_content_object (CAMEL_MEDIUM (receipt), CAMEL_DATA_WRAPPER (body));
> + camel_object_unref (body);
> +
> + receipt_subject = g_strdup_printf ("Delivery Notification for: \"%s\"", message_subject);
I think this should be translated. It also seems pretty long, but what can you do eh.
> + camel_mime_message_set_subject (receipt, receipt_subject);
> + g_free (receipt_subject);
> +
> + addr = camel_internet_address_new ();
> + camel_address_decode (CAMEL_ADDRESS (addr), self_address);
> + camel_mime_message_set_recipients (receipt, CAMEL_RECIPIENT_TYPE_TO, addr);
> + camel_object_unref (addr);
> +
> + addr = camel_internet_address_new ();
> + camel_address_decode (CAMEL_ADDRESS (addr), receipt_address);
> + camel_mime_message_set_from (receipt, addr);
> + camel_object_unref (addr);
> +
> + camel_medium_set_header (CAMEL_MEDIUM (receipt), "Return-Path", "<>");
> +
> + /* Send the receipt */
> + out_folder = mail_component_get_folder(NULL, MAIL_COMPONENT_FOLDER_OUTBOX);
> + info = camel_message_info_new ();
> + info->flags = CAMEL_MESSAGE_SEEN;
> + mail_append_mail (out_folder, receipt, info, em_utils_receipt_done_cb, 0);
> Index: mail/em-folder-view.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
> retrieving revision 1.85
> diff -u -u -r1.85 em-folder-view.c
> --- mail/em-folder-view.c 28 Jul 2004 14:38:50 -0000 1.85
> +++ mail/em-folder-view.c 16 Aug 2004 12:11:32 -0000
> @@ -29,6 +29,9 @@
> #include <sys/stat.h>
> #include <unistd.h>
>
> +#include <netdb.h> /* define MAXHOSTNAMELEN on Solaris */
> +#include <sys/param.h> /* define MAXHOSTNAMELEN elsewhere */
> +
> #include <gtk/gtkvbox.h>
> #include <gtk/gtkbutton.h>
> #include <gtk/gtkvpaned.h>
> @@ -63,6 +66,7 @@
> #include <bonobo/bonobo-ui-util.h>
>
> #include "widgets/misc/e-charset-picker.h"
> +#include "widgets/misc/e-error.h"
>
> #include <e-util/e-dialog-utils.h>
> #include <e-util/e-icon-factory.h>
> @@ -120,6 +124,8 @@
> static void emfv_on_url_cb(GObject *emitter, const char *url, EMFolderView *emfv);
> static void emfv_on_url(EMFolderView *emfv, const char *uri, const char *nice_uri);
>
> +static void emfv_set_seen (EMFolderView *emfv, const char *uid);
hint: instead of having to define a forward reference, its easier just to
define it before its used. maintaining excessive forward references
has provento be painful. but this isn't important.
this stuff (em-folder-view.c) looks fine.
> diff -u -u -r1.171 mail-account-gui.c
> --- mail/mail-account-gui.c 18 Jun 2004 04:43:59 -0000 1.171
> +++ mail/mail-account-gui.c 16 Aug 2004 12:11:38 -0000
Ok, this is problematic. This file is being rewritten in the EPlugin
branch, which will be merged/or used by the time this patch can get
into evolution. Literally 'being' rewritten, it doesn't work yet and
isn't finished being rewritten.
The new code does things very differently. When you wrote this patch
none of the new code even existed yet, so we couldn't ask you do work
on that branch.
So we probably can't use any of the config gui stuff from this patch,
i'm even moving the optionmenu's to use gtkcombobox's.
So maybe we can just split this stuff out and leave it for now, get
the other stuff in with those minor changes, and either i'll just do
the gui as part of the other config work i'm part-way through, or it
can be done later on since its completely separate.
> Index: mail/mail-errors.xml
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/mail-errors.xml,v
> retrieving revision 1.4
> diff -u -u -r1.4 mail-errors.xml
> --- mail/mail-errors.xml 29 Jul 2004 06:47:31 -0000 1.4
> +++ mail/mail-errors.xml 16 Aug 2004 12:12:01 -0000
> @@ -315,5 +315,13 @@
> <primary>Could not connect to {0}. Groupwise account setup is incomplete. You may need to setup the account again</primary>
> </error>
>
> + <error id="ask-receipt" type="question" default="GTK_RESPONSE_NO">
> + <primary>Receipt requested</primary>
> + <secondary>Send message receipt to {0} about message "{1}"?</secondary>
> + <button stock="gtk-no" response="GTK_RESPONSE_NO"/>
> + <button stock="gtk-yes" response="GTK_RESPONSE_YES"/>
> + </error>
Could use puncuation, e.g. primary text should end in a full-stop.
Instead of "Yes" it should have a label of "Send Receipt" instead to
make it more consistent with the hig.
I could be really niggly and suggest the id is "ask-send-receipt", but
who cares!
> </error-list>
>
> Index: ui/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/ui/ChangeLog,v
> retrieving revision 1.398
> diff -u -u -r1.398 ChangeLog
> --- ui/ChangeLog 23 Jun 2004 03:59:03 -0000 1.398
> +++ ui/ChangeLog 16 Aug 2004 12:12:04 -0000
> @@ -1,3 +1,8 @@
> +2004-08-16 ERDI Gergo <
cactus cactus rulez org>
> +
> + * evolution-message-composer.xml: Added new menu item for
> + requesting message receipts when composing a new message
> +
> 2004-06-22 V Ravi Kumar Raju <
vravikr yahoo co uk>
>
> * evolution-addressbook.xml: Remove the Menu Seperator in View
> Index: ui/evolution-message-composer.xml
> ===================================================================
> RCS file: /cvs/gnome/evolution/ui/evolution-message-composer.xml,v
> retrieving revision 1.43
> diff -u -u -r1.43 evolution-message-composer.xml
> --- ui/evolution-message-composer.xml 25 May 2004 20:54:01 -0000 1.43
> +++ ui/evolution-message-composer.xml 16 Aug 2004 12:12:04 -0000
> @@ -31,6 +31,10 @@
> pixtype="stock" pixname="gtk-delete"
> _tip="Delete all but signature"/>
>
> + <cmd name="RequestReceipt" _label="Request Receipt"
> + _tip="Check to get delivery notification when your message is read"
Maybe "Request notification of when your message is read", but again i
dont think i care.
On Mon, 2004-08-16 at 14:17 +0200, ERDI Gergo wrote:
On Thu, 12 Aug 2004, ERDI Gergo wrote:
> So here's a new version with all the discussed improvements and decisions. I
> also modified the code to use a userflag instead of a dedicated
> RECEIPT_HANDLED bitfield.
and by now I've also modified the account editor to include a combo box
for the setting.
--
.--= ULLA! =---------------------. `We are not here to give users what
\ http://cactus.rulez.org \ they want' -- RMS, at GUADEC 2001
`---= cactus cactus rulez org =---'
The smallest handcuff in the world is a wedding ring.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
