*** Bug 326874 has been marked as a duplicate of this bug. ***

Evolution should not notify about new mail, if there ain't one...

Definitely no notification about Junk. IMHO we should trigger notifications only, if the mail will end up in the Inbox. Thus after all filters are processed.

Originally reported for 2.3.x. According to the dupe still valid in 2.5.x.

This may be related: I just noticed that audio mail notification is given not just for new mail, but for moving existing mail from the Inbox to another folder.  IMHO, "New Mail Notification" should apply only to *new* mail, not old mail.

Running evolution-2.5.92.

*** Bug 322016 has been marked as a duplicate of this bug. ***

copying from duplicate bug 322016:

Comment #4 from Karl Relton

Looking at the code, I see that the folder.changed event is passed an
EMEventTargetFolder structure, which is contains a flag EM_EVENT_FOLDER_NEWMAIL
(although curiously this flag is inverted in em_event_target_new_folder() - not
sure why).

Therefore the plugin code in new-mail-notify.c could potentially test this
flag to decide wether to send the dbus message - this would save having to
code a brand new plugin event type.

Is this kind of code discussion welcome here - or should it be on the
evolution-hackers list?

I had a long thread on the mailing lists about this back in Nov/Dec. The solution is not as easy as first thought, and since then I got distracted onto other stuff and have not pursued it.

However, I still believe there is a solution to all this related to the points in the code where filtering happens. This is because right now I have a hacked
evolution which 'fixes' the problem reported in 322016.

The hack is as follows:

- Hack out the code that sends the dbus 'newmail' message from evolution
- Create a dummy filter on incoming mail: this filter triggers on any incoming mail and runs a script which manually sends the dbus 'newmail' message.

The behaviour of this is fine (but doesn't address the newmail-is-junk issue), and thus proves the concept.

The trick is to hard-code this dummy filter in the evolution source code. The
problem here, I think, is that depending on the backend, filtering is triggered
in different parts of the code, i.e. there is no single code point that needs
changing - hence not trivial to solve.

*** Bug 335538 has been marked as a duplicate of this bug. ***

*** Bug 337056 has been marked as a duplicate of this bug. ***

I'm sorry for having made up a duplicate bug (#337056), I'd just would like to repeat my suggestion from there in this thread:

You should be able to set a sound specifically for a folder. Combined with filters, you could have a special sound if your girlfriend writes something (and that mail is put in the folder "sweetheart"), a different sound if your boss writes something (and that mail is put in the folder "nuisances"), and no sound when a mail of a Debian mailinglist comes into the mailinglist folder (which occurs quite often).

You can add a "Play Sound" event to filters.  I've been doing this for a while, but my current version of evolution (2.5.92) is having trouble running the filters intermittently.

The down side to putting the "Play Sound" event in a filter is that it gets repeated for every message that matches the filter, which can sound rather funny  if you have a dozen messages come in all at once and get a dozen overlapping sound bites playing simultaneously.  It would be better to have it play the sound just once (per filter) every time the mail was checked, but I imagine that would take quite a bit of work to implement.

trevin, see bug 333176 – new mail beep should happen once per mail check. :-)

Debian users are also interested in a fix for this problem, see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=345307. Both beep for junk and moving messages should be fixed.

punting.

*** Bug 348294 has been marked as a duplicate of this bug. ***

*** Bug 333176 has been marked as a duplicate of this bug. ***

*** Bug 352551 has been marked as a duplicate of this bug. ***

Same problem here, I'm not really into the Evolution source, but isn't it possible to make a new event. "folder.changed_afterfilters" I think this is also usefull for other plugin's.

*** Bug 354439 has been marked as a duplicate of this bug. ***

Created attachment 72860 [details] [review]
First not fully working patch

I've created a patch for this, but it is not working as expected. On my machine notification is triggered not on every move with the new version. But I could not find out, why it was sometimes fired and sometimes not.

Maybe someone has a better idea how to do this? Maybe I am totally wrong?

not all accounts use movemail

To really solve the various problems reported in this bug will need more than just stalling the notify plugin on movemail, unfortunately.

The only robust method I can see is to move the source of the notification to the code that handles filtering of incoming mail. The problem with this, I believe, is that such code lies potentially in multiple places across the different backends - making it hard to do fix (any fix will need to fixing & testing in each affected place).

A possible way round this is to have evolution have a built-in (i.e. hard-coded)
filter that always applies to incoming mail. This filter would thus be run (just like any other filter) regardless of the backend on incoming mail - and thus newmail would get notified. I'm not sure this would fix the 'unwanted notification on junk mail' issue - because I'm not sure if incoming is first tested to be junk, and then applied to filters if it is not junk.

Thanks Karl,

It was not clear to me which parts are involved there.

I found out, that the code I wrote just disables the notify plugin which sends dbus messages. 

The notification sound is handled in evolution-data-server somewhere with some callback functions.

But there has to be a way to intercept the Drag-Drop-Moving and the Message->Move events and in any way prevent this sound.

And I don't want to turn off the speakers every time I move a mail round.

It sounds as if fixing the current new-mail notification mechanism to avoid triggering on spams (and also on manual manipulation of messages like dragging them between folders), may be difficult.

Perhaps the best solution would be to implement a new mechansim.  I think it might be best to integrate mail notification with the filter system.  You would have a switch on each filter rule which specifies whether or not that rule is executed automatically on new mail (as opposed to the current system, which only allows this to be chosen globally for all rules).  Make a default rule that does the notification (play a sound, pop up a dialog, or put an icon in the notification area) for new mail which is not spam.

I found a possible workaround for this bug:

Create a new Search Folder.  Name it "Unread Mail", and create two search criteria: "Status is not Read" and "Status is not Junk".  Set Find items to "if all criteria are met".  Set the Search Folder Sources to "with all local and active remote folders".

You can add other criteria, e.g., to exclude high-traffic mailing lists, if you like.

Compile mail-notification with Evolution support enabled.  Tell it to monitor the Unread Mail folder.  (Some distros might have Evolution support enabled by default.  Check whether you can select "Evolution" as an option in the Mailbox type pulldown for the mailbox properties under Mail Notification Properties ("mail-notification -p").

Apparently, messages won't show up in Unread Mail until after they have passed the "is not Junk" test.  When they do show up, Mail Notification displays a popup.

Hope this solves it for you too!

The dbus message isn't send for Search Folders, so it is not a workaround for the original problem, but only for mail-notification application.

After some months of inactivity on my part, I've returned to the Evolution
code and the possibilities for this cluster of bugs.

I remain convinced that using a (pseudo)filter is the best approach to fixing
this - a filter than can run on each INCOMING email, after the JUNK test
has been applied (thus it is not run if the mail is junk).

The concept is to have a hard-coded INCOMING filter that on any non-Junk
mail runs a callback back in Evolution. That callback would then do the
appropriate things based on user preferences (i.e. raise popup, play beep/sound,
and send dbus message). It could also employ a throttling mechanism (not
disimilar to existing evolution notification code) to avoid a chain of beeps
when several mails are received at once.

I might be able to cook up a trial patch (but my time for these things is
limited to an hour or two per week). Here is what I would do initially:

- in evolution mail-session.c clone a function like session_system_beep()
  to do the actual notification (based on user prefs, throttling etc.)

- in evolution-data-server camel-filter-driver.c add NotifyFunc along similar
  lines to BeepFunc

- in evolution mail-folder-cache.c remove existing notify code

This in theory should allow the user to manaully add the 'Notify' filter
as an INCOMING filter - which should then work on incoming new mails.
If this works, then further patching would be used to make the Notify
filter a permanent (hard-coded) filter in the system.

Interestingly, in evolution mail-ops.c there is reference to a filter rule
name 'new-mail-notification', but I cannot find any other reference to
this string anywhere else in the evolution or evolution-data-server code.
To me, such a filter rule name looks like just the thing my above patching
would create!

Any comments before I try and start coding?

Make sure it works with an IMAP connection, and that it applies to all new messages and only those messages in the Inbox.  It should catch messages both at evolution's start-up and received while running (during automatic mail check).

Created attachment 77545 [details] [review]
Add a notify filter to evolution-data-server

This is a proof of concept patch - adding a 'notify' filter-action to
evolution-data-server. This patch must be used in conjunction with a patch
to evolution itself to allow this filter-action to be used.

This patch just creates the code for the filter-action - it is then up to the
user to manually add a filter that uses this action to their setup (this is a
temporary measure - eventually the filter would become 'builtin/hardcoded' so
it is always activated.

Created attachment 77546 [details] [review]
Make Evolution mailtool do new new mail notification on notify filter-action

This patch is a proof of concept, and must be used in conjunction with the
patch to evolution-data-server.

It adds the code for the new notify filter-action, and adds this new
filter-action to the list of known actions in Evolution (so it appears in
the edit filters UI).

It removes the new mail notification code from mail-folder-cache.c, and puts
(more or less the same code) in mail-session.c. accessed via the callback for
the new notify filter-action.

The code keeps the throttling mechanism from the original code, i.e. it only
triggers the code if an elapsed time has past, and the tool has become idle.
I increased the elapsed time to 30 seconds (it was previously 5, which seemed
too small).

Note: I currently make no attempt to pass the proper uri for the mail folder to
the new-mail-notification event. Since the uri can't be NULL (I don't think), I
have just used a 'dummy' uri for now. This doesn't matter, since the
folder-name
in the dbus message isn't used to my knowledge.

With these two patches in place, the user can then manually add a filter
on INCOMING mail: I suggest filter on 'status != read' and set the action to
notify. You should then get notification (play beep or sound) and dbus message
only when new mail arrives - but not when junk arrives (the filter is not run
if the mail is already seen as junk).

I have tested this with imap so far - I intend to test with mbox and pop, but
can't test MS-exchange or hula.

With the above two attachments, I have it working with the following types
of mail backend: pop, imap, mbox, mdir, and local delivery. I haven't tried
mh-format, exchange, groupwise or hula - but I would imagine all these work as well since its the same basic system and codepath for each.

I will now attempt to generate patches for the next step, which is to change my new filter to be 'built-in' so it is always executed.

Created attachment 80036 [details] [review]
Evoloution patch to implement notification via filtering

This is a revision of my first patch, which effectively hard-codes a new mail notification into the system. Therefore the need for a user to set up such a filter is no longer (as done in the proof of concept) is no longer necessary.

Created attachment 80037 [details] [review]
Evolution-data-server patch to implement notification via filtering

This patch is actually exactly the same new code as my original path for e-d-s (no 77545), but I have re-cut it against the latest e-d-s version 1.9.5. It adds the notify filter action into the camel filter system.

With the two latest patches (ids 80036 & 80037) to evolution and e-d-s respectively, new mail notification is switched from happening in the folder_changed code to happening as a result of a filter that is added (i.e. hardcoded) into the filter_driver. This filter will run a new filter action 'notify' on incoming mail if it is unseen and not junk. The code run by the notify action does the beeping, playing sound, and sending (via the original plugin)
the dbus message as per user preferences.

The result is that new mail notification ONLY happens when new mail arrives, and when that new mail is NOT junk. If the user has a filter to move the mail to another folder, you will still get the notification.

There are two current pitfalls:
- If the user has a 'stop processing' filter, then the newmail filter will not get executed on mails where processing is stopped.

- If the user has something Imap with server-side-filtering (i.e. in the imap server, not in evolution client), then newmail to
folders other than the Inbox do not get filtered so there is no notification.

While you're hacking on this, any chance you could add the option to feed the header and text of the new message to an external program?  I like sending new messages to a shell script that finds the subject line and reads it aloud using 'festival' (I used to have a script that monitored the incoming message folder in a Maildir directory for this, but it would be much cleaner and more flexible to get Evolution to invoke the script).  Probably there are a lot of other potentially useful things one could do.

Chris

Such functionality already exists in Evolution without me needing to do any more hacking! Simply define your own filter (suggest filter on Status is-not Read)
and set the action to 'Pipe to Program' - which should give you what you want.

Karl

Karl: this is how we originally implemented this feature... I don't remember what all the problems were, but it's got at least these problems:

- it won't work for folders other than "Inbox"

- it won't work unless the user enables filtering for IMAP/MH/Maildir (aka Storage) account types.

Created attachment 85039 [details] [review]
Proposed patch for Evolution - check emails in real_flush_updates

I now propose the attached patch to fix this bug. This approach does
not attempt to notify 'at source' as emails come in (as previous attempts
of mine did), but instead patches the existing mechanism found in
real_flush_updates() in mail-folder-cache.c.

It works by making evo folder_changed() pass a copy of added uids and also a filter thread id into the asynch real_flush_updates thread. This thread can then wait on the filter thread (if set), then check the uids for being unseen, non-junk, and being more recent than the last new mail that arrived. Only
if added messages past this test do we get a new mail notification.

We wait on a filter thread because 'storage' type mailboxes have their
email arrive into the mailbox, and then filtering happens in parallel (in
a new thread). We therefore need to wait for this filtering to complete,
to allow it to possible mark the email as junk, or move it to another
folder.

We have to wait on the filter thread in the async real_flush_updates
thread, otherwise evolution will deadlock. This is because the
filter thread itself creates an update - and the folder_changed event
mechanism locks this new update out until the previous one(s) have
finished.

In order to wait on the filter thread we need a patch to e-d-s which I will
attach in a moment - the two patches are designed to be used together.

I have tested this quite extensively on different types of mailbox. You
only get a notification when new mail really arrives. If the new mail is
filtered into another folder, you will still get notification - based on
the destination folder name. If the newmail is set as 'junk' by filtering
it will not produce a notification. Finally, if you just move an unread
message from one folder to another, you will not get a notification.

So I believe this patch, in conjunction with the e-d-s patch I am about to submit, addresses this bug in its entirety.

Created attachment 85040 [details] [review]
Proposed patch for Evolution-data-server - provide filter thread id

This patches e-d-s (version 1.10) to make a filter thread id visible in
the API via a new camel_get_folder_filter_thread() call. On filter_threads
arising from 'storage' type mailboxes are stored and made available by this
patch. When the thread has finished, the id is reset to 0 to denote no
currently running filter thread.

The patch allows Evolution to know that filtering is currently running on
'storage' type mailboxes, so it can wait before testing newmail.

I think except the last two patches others are absolete. 

Varadhan/Sankar/Matthew: can one of you review this for 2.11.x?

Yes all but last two patches are obsolete. Actually Fejj was kind enough to take a look and sent some comments to me.

The main point was whether we could avoid having an evolution folder_changed action having to wait on the e-d-s filtering. He contended that it would be
better to swallow folder_changed events going to evolution until after the
filtering was done, so that evolution would then have a clean action to deal
with.

Looking at the code I can see what he means, and am happy to attempt this more sophisticated (but better in the long run) approach.

At the moment I am distracted with other work, but do hope to take a look in
a few weeks time.

Karl, great to hear that. Lets hope that the issue gets addressed before 2.11.3 :)

Good news

I have worked up a first cut of my improved patch, catching folder_changed events until after filtering is done. Initial testing on my machine looks okay.

Hopefully within about a week's time I'll get the patch cleaned up ready for
review here on bugzilla.

cool, looking forward to that!

Created attachment 88501 [details] [review]
Proposed patch for e-d-s: hold folder_changed until filtering done

This is a patch for e-d-s, changing the folder_changed handler to block emission to other handles (i.e. the evolution handler) until after filtering is done. When filtering is required, the folder is frozen and the changeset put back on the
frozen_changes list while filtering takes place.

This will mean that evolution itself will not see the folder_changed event until after filtering is done - and so evolution will see the messages in their post-filtered state, in turn allowing evolution to see whether the messages are in fact junk, already deleted etc.

Should patch against e-d-s 1.11.2

Created attachment 88502 [details] [review]
Proposed patch for Evolution - check emails in folder_changed

This patch assumes that e-d-s has also be patched so that evolution will only see a folder_changed event once filtering has taken place.

Know that the messages are now post-filtering, it can check the message flags to see if they are junk, already deleted, as well as already seen and checking against a (crude) timestamp to see if the message really is new to the system.

The field up->new then reports the number of truely new (non-junk, non-deleted etc.) mails.

Should patch against Evolution 2.11.2

looks good Karl, can you email me ChangeLog entries for each of these 2 patches and then I can commit them.


Thanks!

Fejj, thanks for your review and the continuous help to Karl and Karl thanks to your awesome patch commitment :-)

Im planning to push this to 2.11.3 and Fejj, it will be great if you can commit them before (Thatz monday morning). Im sure that Karl would provide the ChangeLogs.

e-d-s patch has been committed as of revision 7794

evolution patch has been committed as of revision 33614

btw, thanks again Karl :)