308148 - read messages in IMAP folder are not filtered

Status: RESOLVED FIXED

(MailNews Core :: Filters, enhancement)

Description

[newsletter2]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.7.11) Gecko/20050728 Mnenhy/0.7.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.7.11) Gecko/20050728 Mnenhy/0.7.1

If I use my IMAP folder INBOX via Webmail, the messages I read become marked
"read". If I afterwards start Mozilla MailNews, the read mails are not touched
by the Mail filter (not Junk filter!).

Reproducible: Always

Steps to Reproduce:
1. Use an IMAP account.
2. Setup a mail filter to handle mails in INBOX, e.g. mark them as "Important".
3. Mark mails in INBOX as "read" with another mail program (while Mozilla is
closed).
4. Open Mozilla and look what the mail filter does.

Actual Results:  
The read mails are not handled.

Expected Results:  
All mails should be handled by the mail filter.

Comment 1

[David :Bienvenu]

This is by design - we don't filter read messages. I understand that you want us
to, but if we did, people would file bugs saying you shouldn't filter read
messages :-)

Comment 2

[newsletter2]

What about a config option for this? A checkbox like "Apply filters also on read
mail"? It's not that obscure to filter read emails, is it? I will not insist if
you say nobody needs it, but I think there are people that need this but perhaps
do not know about bugzilla or don't take the time filing the bug but just go on
to Opera Mail or something...

Please rethink!

Mathis

Comment 3

[David :Bienvenu]

No one has ever asked for it before, whereas when we did filter on read mail, we
got lots of complaints and bugs filed. I'll changed it to an enhancement and add
the helpwanted keyword, since this isn't something that I plan on doing.

Comment 4

[M van Ketel]

If it will not be fixed as a bug, I hope the config function will be added. Because I agree with the OP, that filters should work on all mail. Now I have to manually run the filters every time (after I've used the webmail).

Comment 5

[Alexander Kluss]

I will hope that this bug would be fixed. It's enough to add a default disabled option for "run filter on read mails".
I make it at the moment that I mark all mails which I read with my mobile phon or webmailer mark as unread befor I sync with seamonkey.

Please don't forget the junk filter. All read mails have the unknown state.

Comment 7

[Kent James (:rkent)]

I've had several requests for this in my FiltaQuilla forums, where advanced filter users tend to hang out. This is particularly an issue in the modern world where people tend to read messages on the cell phones.

A hidden preference that could be enabled manually or by an extension would be real easy to do (at http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapMailFolder.cpp#3116)

Is there any opposition to this? I guess I should just file the patch and see if it flies.

Comment 8

[David :Bienvenu]

The main drawback is with a naive implementation filters would also get applied in the following scenarios:

repair folder - filters would get applied on every message in the folder.
undo move/delete - filters would get applied on the undone message
messages moved/copied to inbox - filters would get applied on these messages as well.

So, for example, a move filter might move a message into an other folder. When I move the message back into the inbox, the filter would dutifully move it back to the other folder.

Copy filters would cause multiple copies if fired on a header again. I think tag actions are smart enough not to apply the same tag twice.

Now, it's very possible that this feature is so highly desired for some folks that they would be willing to live with the edge cases. The edge cases could also be worked around with meta data, except in the repair folder case, though at the cost of some db size explosion.

Comment 9

[David :Bienvenu]

let's not talk about reply/forward filter actions :-)

Comment 10

[David :Bienvenu]

Perhaps a simple trick like only applying filters on messages that are the highwater mark for the imap folder would be sufficient.

Comment 11

[David :Bienvenu]

sorry for the spam - but there is an issue that we download the newest XXX message headers first, then older messages, which kinda breaks the highwater idea, unless we use the highwater mark of the last get new mail operation, not the highwater mark of the last header we added to the db.

Comment 12

[Kent James (:rkent)]

I guess this is still ANOTHER example of overloading of the concept of "new", this time asking poor UNREAD to double as a proxy for "really new and therefore don't filter". Unfortunately that has side effects, including to the mobile phone users (which is now virtually everyone) who don't get filters applied to messages that they glanced at on their mobile phones.

I think that the edge cases that you describe are sufficiently bad that it would be risky to just add a simple hidden preference. Metadata is a better answer, such as persisting the processing flags.

Comment 13

[Wayne Mery (:wsmwk)]

(In reply to Kent James (:rkent) from comment #12)
> Unfortunately that has side effects, including to
> the mobile phone users (which is now virtually everyone) who don't get
> filters applied to messages that they glanced at on their mobile phones.
+1

Comment 14

[David :Bienvenu]

(In reply to Kent James (:rkent) from comment #12)

> 
> I think that the edge cases that you describe are sufficiently bad that it
> would be risky to just add a simple hidden preference. Metadata is a better
> answer, such as persisting the processing flags.

I think a better solution is to do the highwater thing I mentioned before. Before we start fetching messages, remember the highwater mark, and whatever headers we fetch that are greater than that value, apply filters to.

Comment 15

[agogin]

(In reply to Kent James (:rkent) from comment #7)
> I've had several requests for this in my FiltaQuilla forums, where advanced
> filter users tend to hang out. This is particularly an issue in the modern
> world where people tend to read messages on the cell phones.
> 
> A hidden preference that could be enabled manually or by an extension would
> be real easy to do (at
> http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/
> nsImapMailFolder.cpp#3116)

Absolutely agree. 
I use FiltaQuilla to Tag my messages in order to sort them between projects. My main email client for send/receive operations is iPad. Thunderbird is kind of useful client for email storage which helps me to organize my work. I open it may be three-four times a week and I need ability to run filters on all messages I've been received in between.  

I don't care if the same filter will run twice or more times at the same message because Tag will not be applied twice anyway.

Comment 18

[David.P]

This bug continues to disimprove for the worse.

UNREAD messages in the local inbox folder are also NOT filtered (by a filter set "B"), if they have been moved from the IMAP inbox to the local inbox before (by another filter set "A").

Steps to Reproduce:
1. Use an IMAP account.
2. Setup a mail filter set (A) that copies new mail from the IMAP inbox to the local inbox.
2. Setup a mail filter set (B) that moves mail from the local inbox to a local folder, called for example "XYZ".
3. Receive new mail in your IMAP account.
4. Open Thunderbird and look what the mail filter does.

Actual Results:  
- New mail from the IMAP inbox is copied to the local inbox (of course only if it is unread) :(
- New unread mail STAYS in the local inbox and is not processed by filter set "B".

Expected Results:  
- New unread mail in the local inbox should be PROCESSED by filter set "B", and thus be moved to the local folder "XYZ".

--
Bonus: try and set up a filter that first COPIES new IMAP mail to another IMAP folder and then MOVES it to a local Thunderbird folder. If several mails arrive simultaneously, Thunderbird will a) process only some of the new mail, and b) will destroy the rest of the mails in trying to c) do the move and the copy at the same time instead of subsequently, and d) on all of the mail simultaneously instead of one after the other.

Conclusion: the Thunderbird mail filter system is a disaster.

Comment 19

[ayleph]

I am also looking for a solution to this issue. My workaround is to "Run Filters on Folder" on my Inbox after viewing my Exchange mail on another device, but I'd love to not have to do that all the time.

Comment 20

[Stephen]

Just adding my vote for finding a solution to this. I can see that it is technically difficult to implement, from the comments above. 

I think a sane approach would for all "relevant" filters to be applied once, and once only, when a mail is first downloaded into Thunderbird, whether read or unread. "Relevant" meaning the filters are defined for the mail's immediate destination. This would avoid most, perhaps all, of the edge cases mentioned by David :Bienvenu. It will not solve the issue mentioned by David P. (comment 18). However, a workaround for the scenario mentioned by David P. is to incorporate the filters in filter set B into filter set A.

Comment 21

[Kent James (:rkent)]

I think that the highwater trick of comment 10 is worth a shot. I'll take the bug for now to keep it on my radar screen, but if someone else would like to try it please ping me.

Comment 23

[Oliver Pfeiffer]

I did change over to Thunderbird some month ago and it took me weeks to find out why some of my emails are not filtered occassionally. I use an IMAP account that is connected from one Thunderbird and many mobile devices. Obviously the filtering should only be performed by Thunderbird if the emails are seen in the inbox for the first time (by Thunderbird). 

Unfortunately if I firstly open a certain email on my smartphone or tablet, this email was flagged as read and Thunderbird doesn't apply my filters anymore.

Strangely the same doesn't happen to the Junk filter which actually works as expected on previously read email. Thus it was quite difficult to understand this issue and finally find this bug report.

So I would highly appreciate anyone who is able to introduce an option to the settings that configures whether the filters shall be applied to new messages or only to new/unread messages.

Thank you so much for your support!

Comment 24

[Bob]

Just forced to move from POP to IMAP, and discovered this feature/bug.
Is there a workaround other than not reading emails elsewhere? Someone mentioned the action to make emails unread, but that's in a filter; would it even run on the unread emails? 
Thanks for helping!

Comment 25

[nabice]

please fix it

Comment 26

[giulia]

Accordingly to these two pages (http://www.adestra.com/resources/top-10-email-clients/ and http://www.emailmonday.com/mobile-email-usage-statistics), 45% of email opens occured on mobile, 36% on desktop and 19% in a webmail client.

INHO, the problem, today, is more frequent than in 2012 and needs a solution.

Giulia

Comment 27

[Kent James (:rkent)]

This is probably not a difficult fix, and I agree a big annoyance, so let's at least try to get it into TB 45.

Comment 28

[giulia]

Thanks

Probably, storing the time of the last imap connection and using it in the next one to identify added messages could help

Giulia

Comment 29

[Jorg K (CEST = GMT+2)]

(In reply to Kent James (:rkent) from comment #7)
> This is particularly an issue in the modern
> world where people tend to read messages on the cell phones.
Agreed!

> Is there any opposition to this? I guess I should just file the patch and
> see if it flies.
No opposition. My wife is upset about all the hours I put into TB and she can't even filter her incoming IMAP e-mail :-(

(In reply to Kent James (:rkent) from comment #27)
> This is probably not a difficult fix, and I agree a big annoyance, so let's
> at least try to get it into TB 45.
Where is it?

Can you please update the line number
http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapMailFolder.cpp#3116
and I can see whether I can add a simple patch.

Comment 30

[Wayne Mery (:wsmwk)]

This is much too late for tb45, because we should assess feedback after it's done before it goes in a release - including mine. I like the idea but I'm on the fence.  For example, my workflow is such that a bug that is marked read from my phone and then gets filtered to another folder is likely to simply get lost and neglected. Which makes me think this should have a hidden or exposed pref.

Comment 31

[Jorg K (CEST = GMT+2)]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #30)
> Which makes me think this should have a hidden or exposed pref.
That's the plan.

Comment 32

[Teodor]

In the last few months I came across a bug (1251932) (not sure if this gets automatically linked, I'm not a frequent writer here) that involves selecting folders for auto-check in addition to Inbox (folder properties - "When getting new messages for this account, always check this folder") that prevents me from using server-side filters to move certain messages to other folders. So I have to use local TB filters for this task.
My TB is the "master" email client. I read my mail on other devices as well (I call them slaves).
So the current bug seriously impedes my use of the email accounts.

In response to Wayne's considerations and use-case:
Phones' email clients are inferior when we talk about features and abilities to filter and dispatching messages. Using a "master" TB on a desktop serves this, and other tasks very reasonably.
If a bug-type message is read on your phone, you presumably are aware of it already. If a desktop installation then moves it to a different folder, it is no different than to count on this to find all such messages in a given folder where these messages go sooner or later (either manually on the phone, or on the desktop by constantly running and auto-checking TB, just like I am). The worst case scenario in my situation would be few messages residing in Inbox until the "master" client kicks in and moves them to the appropriate folder (only if I have read it before TB kicks in). If TB had moved the new message to the other folder already before I had the chance to check on the phone, then the phone would just notify me of the message in the other folder (there I can setup autocheck for other folders as well, with no drama or bugs).

A hidden preference to apply filters on read mail as well as unread in advanced preferences would harm nobody - by default it would be disabled. Still, nowadays I guess a lot of people would rather have it enabled (if they are properly notified of its presence) if they count on their TB filters to move mail around.

Comment 33

[Kent James (:rkent)]

I have a working patch for this, the holdup has been that I could not get a test to work (and it was giving unexplained behavior even without the actual change.) I'll post the patch as-is when I return home.

Comment 34

[Kent James (:rkent)]

Attachment: Filter on highwater, behind a hidden preference

Comment 35

[Kent James (:rkent)]

Comment on attachment 8729105 [details] [diff] [review]
Filter on highwater, behind a hidden preference

I'd like to try to land this for TB 45 behind a hidden preference, and there is still time for that if we push. Why? I think that we need some end-user testing of the concept of using highest UUID as the filtering flag that will be easier in release. Plus I would really like to use this myself!

Comment 36

[Jorg K (CEST = GMT+2)]

Comment on attachment 8729105 [details] [diff] [review]
Filter on highwater, behind a hidden preference

Review of attachment 8729105 [details] [diff] [review]:
-----------------------------------------------------------------

I feel honoured by the review request, but I don't think I can review in mailnews/imap ;-(
However, I've looked at the patch and it looks good. If you think I should r+ it, please address the issues mentioned below.
I have also tested the patch and it works: I read a new message on my Android, then started TB and it got filtered/moved. A pre-existing message in the IMAP inbox to which the filter also applied was not moved. So this is great to have.

Further comments:
Can we please change the very geeky name of the preference: mail.imap.filter_on_highwater to something more user-friendly? Why not call it what is was implemented for: mail.imap.filter_new_messages?
Also, can be please add a default for this so the user can find it and doesn't have to add it.
TB is riddled with undocumented preferences, so let's not add one more.

I'll just give you some examples:
mailnews.disable_format_flowed_for_cjk - I killed that one.
mail.force_user_charset
mail.use_content_location_on_send

Comment 37

[giulia]

Hi

Let me know when a version to play with will be available

As new messages are already filtered, I suggest to name the preference: mail.imap.filter_read_messages

+1 to have inside declared preferences and set to false 

Thanks for addressing this issue

Giulia

Comment 38

[Jorg K (CEST = GMT+2)]

(In reply to giulia from comment #37)
> As new messages are already filtered, I suggest to name the preference:
> mail.imap.filter_read_messages
Actually, new messages *aren't* filtered if they are read. Perhaps: mail.imap.filter_new_read_messages
(If we keep discussing this, Kent will give us his "highwater" ;-( )

Comment 39

[giulia]

ok

No matter on the preference name. The important aspect is the functionality and, when it will be available, I'll write an addon to help user setting/unsetting it :)

Giulia

Comment 40

[Wayne Mery (:wsmwk)]

(In reply to Jorg K (GMT+1) from comment #38)
> mail.imap.filter_new_read_messages

+1

Comment 41

[Emily Bowman]

Glad to see this going at last, it's affected me for years.

David P's comment #18 is actually my exact scenario, restarting filters after moving, but I don't think that's really related to this bug. (Except for the pre-read messages that don't move over.) That's a complex enough scenario that it deserves a new bug or at least an add-on.

If this gets into 45 I'll help test for you guys.

Comment 42

[Kent James (:rkent)]

Attachment: use mail.imap.filter_on_new as pref

Comment 43

[:aceman]

Comment on attachment 8730860 [details] [diff] [review]
use mail.imap.filter_on_new as pref

Review of attachment 8730860 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, I can't test this much but I trust Jorg did it.
Didn't you plan to have a tests for this? Or did it not pan out?
But the logic seems fine, thanks.

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +3092,5 @@
> +    bool doFilter = filterOnHighwater ?
> +      // Filter on largest UUID and not deleted.
> +      m_curMsgUid > highestUID && !(msgFlags & nsMsgMessageFlags::IMAPDeleted) :
> +      // Filter on unread and not deleted.
> +      !(msgFlags & (nsMsgMessageFlags::Read | nsMsgMessageFlags::IMAPDeleted));

I'm not sure this is actually readable. The interleaved comments... Maybe you could extract out msgFlags and !nsMsgMessageFlags::IMAPDeleted as they seem common in both cases.

::: mailnews/mailnews.js
@@ +102,5 @@
>  pref("mail.imap.check_deleted_before_expunge", false);
>  pref("mail.imap.expunge_option",            0);
>  pref("mail.imap.expunge_threshold_number",  20);
>  pref("mail.imap.hdr_chunk_size", 200);
> +// Should we filter imap messages based on highwater (highest UUID) instead of unread?

You want to say something like "new messages since the previous highest UUID seen" ?

Comment 44

[Jorg K (CEST = GMT+2)]

Comment on attachment 8730860 [details] [diff] [review]
use mail.imap.filter_on_new as pref

Review of attachment 8730860 [details] [diff] [review]:
-----------------------------------------------------------------

All good, just discussing final nits here.

::: mailnews/imap/src/nsImapMailFolder.cpp
@@ +3080,5 @@
>    // if this is not the Inbox folder.
>    if (mFlags & nsMsgFolderFlags::Inbox || m_applyIncomingFilters)
>    {
> +    // Use highwater to determine whether to filter?
> +    bool filterOnHighwater = false;

Personally, I don't like the term "high water". Is this a standard term used in this scenario? What's wrong with filterOnNew? (And "new" means, higher UUID than last highest one seen).

@@ +3092,5 @@
> +    bool doFilter = filterOnHighwater ?
> +      // Filter on largest UUID and not deleted.
> +      m_curMsgUid > highestUID && !(msgFlags & nsMsgMessageFlags::IMAPDeleted) :
> +      // Filter on unread and not deleted.
> +      !(msgFlags & (nsMsgMessageFlags::Read | nsMsgMessageFlags::IMAPDeleted));

I find this quite readable. I little hard to pull out!nsMsgMessageFlags::IMAPDeleted. It would also lose clarity.

Comment 45

[Jorg K (CEST = GMT+2)]

s/I little/A little/

Comment 46

[Kent James (:rkent)]

(In reply to Jorg K (GMT+1) from comment #44)
> Comment on attachment 8730860 [details] [diff] [review]
> use mail.imap.filter_on_new as pref
> 
> Review of attachment 8730860 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> All good, just discussing final nits here.
> 
> ::: mailnews/imap/src/nsImapMailFolder.cpp
> @@ +3080,5 @@
> >    // if this is not the Inbox folder.
> >    if (mFlags & nsMsgFolderFlags::Inbox || m_applyIncomingFilters)
> >    {
> > +    // Use highwater to determine whether to filter?
> > +    bool filterOnHighwater = false;
> 
> Personally, I don't like the term "high water". Is this a standard term used
> in this scenario? What's wrong with filterOnNew? (And "new" means, higher
> UUID than last highest one seen).

Highwater is the term that is used in the mailnews code for the highest seen UUID. I did not pick the term, but it is all over the place in mailnews code.

"new" by contrast has a zillion different meanings within mailnews code. "filterOnNew" is virtually meaningless as a practical internal term. We are not talking UI here, we are talking internal code.

I really don't understand the level of interest in this naming, it is really not that important.

Comment 47

[Jorg K (CEST = GMT+2)]

(In reply to Kent James (:rkent) from comment #46)
> Highwater is the term that is used in the mailnews code for the highest seen
> UUID. I did not pick the term, but it is all over the place in mailnews code.
Understood.

> I really don't understand the level of interest in this naming, it is really
> not that important.
Don't worry about it then. Let's land it!

Comment 48

[:aceman]

Is this the same?
      // Filter on largest UUID OR Filter on unread.
    bool doFilter = filterOnHighwater ?
      (m_curMsgUid > highestUID) : !(msgFlags & nsMsgMessageFlags::Read)

doFilter &&= !(msgFlags & nsMsgMessageFlags::IMAPDeleted);

(In reply to Kent James (:rkent) from comment #46)
> "new" by contrast has a zillion different meanings within mailnews code.
> "filterOnNew" is virtually meaningless as a practical internal term. We are
> not talking UI here, we are talking internal code.
> 
> I really don't understand the level of interest in this naming, it is really
> not that important.

I think you can keep the internal naming so it is consitent. Only the user facing pref name is important.

Comment 49

[Kent James (:rkent)]

Attachment: patch to land (with changed comment)

Comment 50

[Kent James (:rkent)]

Comment on attachment 8731321 [details] [diff] [review]
patch to land (with changed comment)

http://hg.mozilla.org/releases/comm-aurora/rev/5e98424027a2
http://hg.mozilla.org/releases/comm-beta/rev/d4bf9b03d79d

Comment 51

[Kent James (:rkent)]

Comment on attachment 8731321 [details] [diff] [review]
patch to land (with changed comment)

http://hg.mozilla.org/releases/comm-aurora/rev/5e98424027a2
http://hg.mozilla.org/releases/comm-beta/rev/d4bf9b03d79d

Comment 52

[Kent James (:rkent)]

This bug adds the backend support as currently implemented, preferenced off. We should use a new bug to either enable by default, or add UI for the choice if needed.

Comment 53

[Kent James (:rkent)]

Comment on attachment 8731321 [details] [diff] [review]
patch to land (with changed comment)

[Triage Comment]

Comment 54

[:aceman]

Maybe the pref should be per-account (server).

Comment 55

[Kent James (:rkent)]

http://hg.mozilla.org/releases/comm-esr45/rev/210bbb0b108e(In reply to :aceman from comment #54)
> Maybe the pref should be per-account (server).

I fully expect that after some testing, we will make this the default. We don't need to give it additional complexity like this.

Comment 56

[giulia]

As promised, the super-simple addon ExtendIMAPFilters is available at this link (https://app.box.com/s/hb1x5jcg4htcfov0ninifx4z824xf9r3).

The new preference will be set to true at addon installation time while will be set to false at addon removal. 

Giulia

Comment 57

[Jorg K (CEST = GMT+2)]

Why isn't that restartless with a bootstrap.js? Take a look at a simple restartless one like:
https://addons.mozilla.org/en-us/thunderbird/addon/dictionary-for-recipient/

Comment 58

[giulia]

(In reply to Jorg K (GMT+1) from comment #57)

Thanks for the suggestion

Giulia

Comment 59

[giulia]

The bootstrapped version: https://app.box.com/s/hb1x5jcg4htcfov0ninifx4z824xf9r3

Giulia

Comment 60

[Jorg K (CEST = GMT+2)]

Much nicer, don't you think? Thanks!

Comment 61

[giulia]

Yep !!

Thanks Jorg

Comment 62

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/73edb3ab0b80

Comment 63

[giulia]

Here it is: https://addons.mozilla.org/en-US/thunderbird/addon/extendimapfilters/

Giulia

Comment 64

[Emily Bowman]

I can confirm that this is now working as expected for Thunderbird 45 revision 5929.

Comment 65

[photoboard]

Fixed. 
http://photoboard.us