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 632153 - Set X-Evolution-Source header consistently
Set X-Evolution-Source header consistently
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Mailer
2.30.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
evolution[imapx]
Depends on:
Blocks:
 
 
Reported: 2010-10-14 14:59 UTC by Brian J. Murrell
Modified: 2011-09-13 14:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add X-Evolution-Source: header (354 bytes, patch)
2010-10-14 14:59 UTC, Brian J. Murrell
none Details | Review
add X-Evolution-Source: header - v2 (316 bytes, patch)
2010-10-15 18:18 UTC, Brian J. Murrell
reviewed Details | Review
eds patch (1.17 KB, patch)
2011-09-13 14:09 UTC, Milan Crha
committed Details | Review

Description Brian J. Murrell 2010-10-14 14:59:47 UTC
Created attachment 172361 [details] [review]
add X-Evolution-Source: header

In some previous releases of Evolution, there was an X-Evolution-Source: header added to e-mails showing which folder/account they were in.  That seems to have gone missing in (some) 2.30.x release(s).

I have a patch for this but I have no idea how correct it is.
Comment 1 Matthew Barnes 2010-10-14 21:03:38 UTC
Might be better to use camel_mime_message_set_source().

Perhaps that's what the FIXME comment is implying in the IMAP provider:

/* FIXME, this shouldn't be done this way. */
camel_medium_set_header (CAMEL_MEDIUM (msg), "X-Evolution-Source", store->base_url);
Comment 2 Brian J. Murrell 2010-10-15 11:08:50 UTC
So given my original patch:


--- camel/providers/imapx//camel-imapx-folder.c	2010-07-27 10:20:44.890600063 -0400
+++ camel/providers/imapx//camel-imapx-folder.c.patched	2010-07-27 10:20:36.082893782 -0400
@@ -224,6 +224,9 @@
 		camel_object_unref(stream);
 	}
 
+	if (msg)
+		camel_medium_set_header (CAMEL_MEDIUM (msg), "X-Evolution-Source", istore->base_url);
+
 	return msg;
 }

It would seem you are suggesting to use instead:

--- camel/providers/imapx//camel-imapx-folder.c	2010-07-27 10:20:44.890600063 -0400
+++ camel/providers/imapx//camel-imapx-folder.c.patched	2010-07-27 10:20:36.082893782 -0400
@@ -224,6 +224,9 @@
 		camel_object_unref(stream);
 	}
 
+	if (msg)
+		camel_mime_message_set_source (msg, istore->base_url);
+
 	return msg;
 }

Is my understanding correct?
Comment 3 Matthew Barnes 2010-10-15 11:53:59 UTC
Right.  camel_mime_message_set_source() strips out extraneous info from the URL.

I'll let chen sign off on this since he's the IMAPX guy.
Comment 4 Brian J. Murrell 2010-10-15 18:18:24 UTC
Created attachment 172446 [details] [review]
add X-Evolution-Source: header - v2

Updated per suggestion.

Tested and works with 2.30.3
Comment 5 Milan Crha 2010-10-26 12:07:31 UTC
I'm only wondering, what is this header good for, for you? Seeing the camel sources only minority of providers are using it, namely IMAP and GroupWise, other providers doesn't work with it at all. The other usage is when filtering messages, and for these is the source URL set automatically, thus there is no need for this header to be included, at least from Camel's point of view.

The reason why the header disappeared is not a code change, but that you moved from IMAP to IMAP+ with your account. :)
Comment 6 Brian J. Murrell 2010-10-30 19:02:12 UTC
(In reply to comment #5)
> I'm only wondering, what is this header good for, for you?

I have a customized "spamassassin" and what it does depends on which account received the spam.  So when I click the Junk or Not Junk, my custom "spamassassin" script is run and gets the e-mail on stdin.  In that script I can tell which of many accounts (i.e. INBOXes) received the spam based on this header.

> Seeing the camel
> sources only minority of providers are using it, namely IMAP and GroupWise,
> other providers doesn't work with it at all. The other usage is when filtering
> messages, and for these is the source URL set automatically, thus there is no
> need for this header to be included, at least from Camel's point of view.

Is the source URL visible to an external script that is called in place of "spamassassin" when [Not] Junk is clicked?
 
> The reason why the header disappeared is not a code change, but that you moved
> from IMAP to IMAP+ with your account. :)

Indeed.  IMAP+ failed to implement it.
Comment 7 Milan Crha 2010-11-01 08:58:42 UTC
(In reply to comment #6)
> Is the source URL visible to an external script that is called in place of
> "spamassassin" when [Not] Junk is clicked?

I searched the code and this information isn't available during the junk actions. I would go with your patch, but it seems to me as not the right fix. The main reason is that the X-Evolution-Source header isn't maintained consistently, so I can take email from the IMAP account and copy it to On This Computer/Inbox and it is still set to the source as it is being in imap folder, but it is in the local Inbox now.

I would rather see a patch to update source header when doing these [Not] Junk actions, and set it back to its previous value when done with the respective action.
Comment 8 Brian J. Murrell 2011-01-19 05:10:35 UTC
(In reply to comment #7)
> 
> I would go with your patch, but it seems to me as not the right fix.
> The main reason is that the X-Evolution-Source header isn't maintained
> consistently,

Indeed, it's not.  It's not maintained in the imap+ code for example but is maintained in the older imap code.  So even without my patch application is consistent.  With my patch it's only equally inconsistent as it is with the old imap provider.  i.e. my patch to apply the header in the imap+ provider is no worse than currently exists in the old imap provider.

> so I can take email from the IMAP account and copy it to On This
> Computer/Inbox and it is still set to the source as it is being in imap folder,
> but it is in the local Inbox now.

Perhaps.  I have not tested it.  I don't actually have any local Inbox accounts.  But even if this is the case, it would also be the case when moving mail from an old imap provider account to a local Inbox.
 
> I would rather see a patch to update source header when doing these [Not] Junk
> actions, and set it back to its previous value when done with the respective
> action.

That sounds like a good idea, however I'm afraid I am nowhere near familiar enough with the code and the data objects in play at the time to code this up and it seems like nobody with the knowledge is interested.  :-(

Would it really be so bad to apply my patch even though it's inconsistent with other providers, it's consistent with the (old) imap provider (at least).
Comment 9 Milan Crha 2011-09-13 14:09:13 UTC
Created attachment 196379 [details] [review]
eds patch

for evolution-data-server;

Thinking of it, it's better to always update the message's source when the message is read from a folder. In this case the header value will be always correct. Also note that the value changes in 3.1.x, it doesn't hold account's base URL, but its UID, which can be read from GConf key /apps/evolution/mail/accounts (or just pick one message, open message source and copy&paste X-Evolution-Source header value to your script).
Comment 10 Milan Crha 2011-09-13 14:10:47 UTC
Created commit 5b4a450 in eds master (3.1.92+)