95193 - no indication in root folder of new mail in subfolder

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[RFLazaro]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.3+) Gecko/20010812
BuildID:    2001081208

whenever i retrieve mail and if it is filtered and moved to a subfolder the root
folder of that subfolder does not have any indication that there is new mail in
its subfolder

the root folder is collapsed

Reproducible: Always
Steps to Reproduce:
1. try to creata a subfolder of at least three levels
2. create mail filter which moves the message to that subfolder
3. create a message which matches the filter and send it to yourself
4. collapse the root folder
5. get the new message


Actual Results:  no indication in the root folder that there is a new message

Expected Results:  if the root folder is collapsed and one of it subfolders has
a new message the root folder must have indication that there is a new message

Comment 1

[Keyser Sose]

Marking NEW.

Comment 2

[Olav Vitters]

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

Comment 3

[Jonas Jørgensen]

Marking 4xp.

Comment 4

[Matthias Firner]

This is especially annoying when moving mail to a local folder.

Furthermore, if a message is filtered to a local folder, the inboxes' "new mail" indicator keeps being set, even though there is no new mail in this inbox.

Comment 5

[Wayne Mery (:wsmwk)]

related to bug 313056

Comment 6

[alta88]

there are several f+ for this in bug 377507, but it's not moving and scope is unread messages (rather than new messages).  12+ years is long enough..

Comment 7

[alta88]

Attachment: newmail.patch

Comment 8

[alta88]

As a note, this will indicate new messages in a closed account folders subfolders, after biffstate is reset.  It doesn't apply to non account folders; it would be necessary to use getNewList() recursively to set the property.

Comment 9

[alta88]

Comment on attachment 8371606 [details] [diff] [review]
newmail.patch

Nope, getNumNewMessages() only pertains to new messages subsequent to the last biff, and mNumNewBiffMessages is not cleared upon leaving a folder.  Back to the drawing board.

Comment 10

[alta88]

Attachment: newmail.patch

this works better.

Comment 11

[Richard Marti (:Paenglab)]

Comment on attachment 8372453 [details] [diff] [review]
newmail.patch

I tried 6 levels of subfolders and the new mail was propagated up to the top/closed folder.

Comment 14

[Ulf Zibis]

Can someone add "datalossy" to the whiteboard here?
See also: bug 885344

Comment 15

[alta88]

Attachment: newmail.patch

tweak to exit sooner.

Comment 16

[Magnus Melin [:mkmelin]]

Comment on attachment 8373726 [details] [diff] [review]
newmail.patch

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

::: mailnews/base/public/nsIMsgFolder.idl
@@ +253,5 @@
>    attribute boolean hasNewMessages;
>  
>    /**
> +   * Does this folder or any subfolder have new messages.
> +   */

I know it's like this elsewhere in this file too, but can we make it "Indicates whether this folder or any of its subfolders have new messages."

You should also rev the interface uuid

@@ +437,5 @@
>    attribute boolean charsetOverride;
>    attribute unsigned long biffState;
>  
>    /**
> +   * The number of new messages since this folder's last biff.

Is it really since last biff? New!=Unread

::: mailnews/base/util/folderUtils.jsm
@@ +47,5 @@
>   * for all of the common properties that css styling is based off of.
>   *
> + * @param nsIMsgFolder aFolder  the folder whose properties should be returned
> + *                               as a string
> + * @param bool aOpen            true if the folder is an open container

what's an open container? do you mean if it the subfolder tree is expanded?

Comment 17

[alta88]

(In reply to Magnus Melin from comment #16)
> Comment on attachment 8373726 [details] [diff] [review]
> newmail.patch
> 
> Review of attachment 8373726 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mailnews/base/public/nsIMsgFolder.idl
> @@ +253,5 @@
> >    attribute boolean hasNewMessages;
> >  
> >    /**
> > +   * Does this folder or any subfolder have new messages.
> > +   */
> 
> I know it's like this elsewhere in this file too, but can we make it
> "Indicates whether this folder or any of its subfolders have new messages."
> 

One never knows to what extent old cruft or poor comments should be driveby fixed.  Sure.

> You should also rev the interface uuid
> 

yep.

> @@ +437,5 @@
> >    attribute boolean charsetOverride;
> >    attribute unsigned long biffState;
> >  
> >    /**
> > +   * The number of new messages since this folder's last biff.
> 
> Is it really since last biff? New!=Unread
> 

So this is an interesting thing that I discovered, as I initially thought the most useful api should also return counts.  Yes, in fact.  There are 2 types of 'new', new since last biff (nsMsgDBFolder::mNewMessages) and new as in folder never viewed.  The 2 are not the same, and there are some new message notify bugs since people want notify to only include new since last biff and the code doesn't get it from mNewMessages).

You can see this on an account folder; after the first biff with new messages mNewMessages is > 0 so there's an indicator.  On immediate second biff, with no new messages, mNewMessages = 0 and the indicator is gone.

The number of new as in never visited/seen messages should be cached like mNewMessages, as this is quite useful and too expensive to be gotten for treeView from the db.  But it's a larger rework of new messages overall. 

> ::: mailnews/base/util/folderUtils.jsm
> @@ +47,5 @@
> >   * for all of the common properties that css styling is based off of.
> >   *
> > + * @param nsIMsgFolder aFolder  the folder whose properties should be returned
> > + *                               as a string
> > + * @param bool aOpen            true if the folder is an open container
> 
> what's an open container? do you mean if it the subfolder tree is expanded?

Yes.  I guess container is Places terminology..

Comment 18

[alta88]

To clarify, GetNumNewMessages returns mNumNewBiffMessages, and mNewMessages is a bool not a count.

Comment 19

[alta88]

Attachment: newmail.patch

Comment 20

[Ulf Zibis]

First thanks, that this bug comes into flow.

(In reply to alta88 from comment #17)
> So this is an interesting thing that I discovered, as I initially thought
> the most useful api should also return counts.  Yes, in fact.  There are 2
> types of 'new', new since last biff (nsMsgDBFolder::mNewMessages) and new as
> in folder never viewed.  The 2 are not the same, and there are some new
> message notify bugs since people want notify to only include new since last
> biff and the code doesn't get it from mNewMessages).

With bug 763534 I was thinking of "new as in folder never viewed". If you solve it different, then it is not a dupe of this bug. So my voter here is for "new as in folder never viewed".

Comment 21

[alta88]

This bug solves 'new never viewed' in a closed account or subfolder.

Comment 22

[Magnus Melin [:mkmelin]]

Comment on attachment 8376466 [details] [diff] [review]
newmail.patch

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

Looks ok to me!

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/4654788403f6

Comment 24

[:aceman]

Magnus, is your review valid and enough for the /mailnews part too?

Comment 25

[alta88]

(In reply to :aceman from comment #24)
> Magnus, is your review valid and enough for the /mailnews part too?

Do you fashion yourself as some sort of roving police force?  You are not a peer of any module and have no governance authority; this sort of noise is appreciated by no one.

Clear?

Comment 26

[Magnus Melin [:mkmelin]]

(In reply to :aceman from comment #24)
> Magnus, is your review valid and enough for the /mailnews part too?

Technically no. OTOH, tb is the only consumer of this mailnews code.