Closed Bug 370324 Opened 17 years ago Closed 13 years ago

"Recent" folder context menu doesn't seem to work, shows wrong folders for copy and move

Categories

(MailNews Core :: Backend, defect)

Product:
MailNews Core
Component:
Backend
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 5.0b1

People

(Reporter: ggerard, Assigned: Bienvenu)

References

(
URL
)

Details

(Whiteboard: [gs])

Attachments

(4 files, 6 obsolete files)

list of folders move>recent differs from copy>recent
40.73 KB, image/png
Details
more wip
19.63 KB, patch
Details | Diff | Splinter Review
fix addressing comments [Checked in: Comment 92 & 105]
16.32 KB, patch
neil
: review+
Details | Diff | Splinter Review
make menu counts relative [Checked in: Comment 104 & 106]
5.11 KB, patch
standard8
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.9) Gecko/20061206 Firefox/1.5.0.9
Build Identifier: version 2 beta 2 (20070116)

I moved a message from my inbox and filed it under a mailing list folder I have for keeping track of Apple's ADC announcements. imap://moi@mail.myserver.com/lists/apple.com/adc

I then went to move another message to the same folder. I thought I'd use recents but it showed me 12 Inbox folders (I have 12 accounts so I suppose that's at least right by count...) and other folders, but no "adc" folder.

Two issues, probably separate...

1. "adc" is missing. I have a a couple filters setup and the only recents I see are also in my filters. In fact, there's no folder other than Inbox and Trash in the recents lists that I've travelled to manually in the past 10 days.

2. Regarding the 12 "Inbox" entries shown. I could not tell which account they were associated with. Very confusing. Since I tend to keep similar folder names on different accounts, this goes beyond inbox for me.

Reproducible: Always

Steps to Reproduce:
See details.
Actual Results:  
1. I expected to see the folder which I'd just moved or viewed to show up in recents.
2. I'd expect to see the IMAP path or something similar.
2. is bug 351692.
Summary: "Recents" folder doesn't seem to work → "Recent" folder doesn't seem to work
Yes, recent does need a big tweak.  It is listing :
* rss feeds that have no activity other than new articles
* folders whose only message movement is from filters
* folders based on message volume, not by most recent activity (described in comment 0)

all of which adds up to not being useful.  tested on trunk
Status: UNCONFIRMED → NEW
Component: General → Mail Window Front End
Ever confirmed: true
QA Contact: general → front-end
Summary: "Recent" folder doesn't seem to work → "Recent" folder context menu doesn't seem to work, shows wrong folders for copy and move
Version: unspecified → 2.0
also listed:
* trash special folder (not needed because delete~=move and no one copies to trash)
* junk special folder (similarly)
* inbox special folder (I'm on the fence)

times N accounts for each of the above
any folder that receives new messages is considered "recently used". If I have new mail filtered into folders, I want those folders to be considered recently used because I want to be able to read my new messages from my mru folder view. Message volume has nothing to do with MRU.

I agree that Trash shouldn't be listed, nor Junk, and they aren't for me.
so context menu is only updated if message was "new"?  in that case recently used will never list a folder that was the object of an action (filter, d&d, context move/copy) on "read" message. unfortunately it's easy to assume a folder listed as "move again" or "copy again" on the context menu will migrate to the recent list.  

Still, a context menu folder list that is mru of only new messages is, for me, 99% not useful. The messages left in my inbox or any other folder are not likely to be destined to a filtered folder.  

as for junk and trash, I deleted panacea.dat and restarted TB
* this cleared the recent list
* trash is gone and I deleted an unread message and the trash folder does not appear on the context list
* junk folder is gone and jmc has been off for about a week, so it should never come back (though it should not have been there anyway)

perhaps these got listed through some oddity of me going back to an old version of TB.

bienvenu irc: "we do store the mru date in panacea.dat, as well as in the msg db. but if you delete panacea.dat, we should recreate it from the info in all the db's."

  I did not find this to be true.

"it might be a problem with the context menu not getting updated correctly
as opposed to a problem with the mru folder view"

  on restart "recent folders" view did not get populated with past data from the dbs.  recent list in context menu matched the list of the recent folders view.
with an unread message in trash folder, I selected all messages and did shift delete -> trash folder now appears in context menu recent list. I repeated this with a second imap account, now two trash folders in context list.  

did same with pop account and the pop trash folder did *not* appear in the context list.
Another piece of the action: When you create new folders on one account, they show up in "recent" directly after creation, but not in the usual "move to" list for that account.
After restart of TB, the folders have disappeared from the "recent" list, but don't appear again when a message is moved or copied into, like mentioned above.

Also I found, that the "move to" and "copy to" lists are different. Folders messages have been moved to do not appear in the "copy to" context list. Is this intended? For me, this should be the same list in both. 
I'd vote for RSS  feeds not being listed either, doesn't seem very useful.
Bear in mind that to fix this, you may have to use a different data source for the recent folder menu, since we want things like inboxes and folders that have messages filtered into them in the mru folder view in the folder pane.
Using one mru list for the recently viewed folders and another mru list for the recently Moved/Copied To folders makes sense to me.
This is one of my (few) frustrations with Thunderbird.  I very much want to see moved/copied to in the MRU list.  And they should appear if I move an already read message in.  Thanks. 
This issue seems to be describing a variety of symptoms, and I'm not sure if any of the descriptions directly match what I'm experiencing.  If I move a message by dragging it from the inbox to the icon for the target folder, that folder shows up on the context Move To / Recent list.  If I move a message from the inbox to a folder by drilling down in the context Move To / Account Name / Folder Name / Subfolder Name ... lists the target folder does *not* show up on the Move To / Recent list (but it does show up on the context menu as the 'Move to "[Target Folder]" Again' command).  Not only is this behavior incorrect (if I move a message to a target folder by the second method more recently than any move performed by a direct drag to a folder icon, then that most recent move should be added to the Move To / Recent list, bumping off a folder with the oldest most recent move if necessary) but it's less useful than if the bug had been reversed (ignoring direct drags to a folder but remembering moves performed by drilling down to find the folder in the hierarchy from the context menu): if I already have the icon visible for folders to which I want to drag messages I have much less need for the Recent shortcut than for folders whose icons aren't visible and for which I end up drilling down in the hierarchy from the context menu.  I'm not suggesting that we should ignore moves performed by either method just so we can preserve the bug ecosystem. :-)  I'm just pointing out how annoying the current behavior is.  If what I'm describing is distinct from the bug represented by this issue, let me know and I'll file another.
Recent folders is a well-intentioned feature that teases users with it's half-utility. It's a huge help when it works but it fails so often that it would be less cognitive effort if it wasn't there. Either get rid of it or fix it. If you want this fixed, VOTE ( https://bugzilla.mozilla.org/votes.cgi?action=show_user&bug_id=370324#vote_370324 )for this issue!!
Bob in comment #14
> ... 
> If what I'm describing is distinct
> from the bug represented by this issue, let me know and I'll file another.

Bob, Gerard isn't totally clear in comment 0 but I think you are describing item 1, which may be the same as bug 369862.  This problem also plagues me - only first level folders are listed in "recent". And even then some first level folders don't get listed (seems to be ones that contain other folders).

Gerard, does this describe your problem?

All, if the above does not describe your issue and none of the bugs in this query [1] match your need then please file a new well-worded bug. Thanks.

[1] https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=recent+&long_desc_type=allwordssubstr&long_desc=folder&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&resolution=DUPLICATE&resolution=---&emailreporter1=1&emailtype1=substring&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=substring&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=product&type0-0-0=equals&value0-0-0=thunderbird&field0-0-1=component&type0-0-1=substring&value0-0-1=mailnews&field1-0-0=short_desc&type1-0-0=anywordssubstr&value1-0-0=copy+move+folder
OS: Windows XP → All
Hardware: PC → All
(In reply to comment #16)
> Bob, Gerard isn't totally clear in comment 0 but I think you are describing
> item 1, which may be the same as bug 369862.  This problem also plagues me -
> only first level folders are listed in "recent". And even then some first
> level folders don't get listed (seems to be ones that contain other 
> folders).

Well, for me the level of the folder doesn't seem to have any bearing on whether it will show up in the context Move To / Recent list: the list I see includes folders from all the levels I maintain.

> Gerard, does this describe your problem?
> 
> All, if the above does not describe your issue and none of the bugs in this
> query [1] match your need then please file a new well-worded bug. Thanks.

I have gone through that list carefully, and none of the other issues on that list describes the problem I'm experiencing more closely than this one.  The title of this issue is exactly right, though some of the comments appear to wander off into other problems.  I have come to the conclusion that I don't understand enough about the inner workings of Thunderbird to craft a new issue which wouldn't run the risk of just **** off some developers as they tagged mine a duplicate of this (or some other existing bug).  In fact, I'm no longer even able to consistently reproduce the pattern of behavior I described in comment #14: TB's decision as to whether to put a folder on the Move To / Recent list looks to be purely random, as far as I can tell.  Is there any documentation describing how it's *supposed* to work?  That would be a useful starting place for (a) discussions about whether the design is optimal and (b) reporting discrepancies between expected and observed behavior.

This problem certainly seems to be a serious annoyance to a lot of users; I'm dismayed to see it still flagged as New after almost a year, with no evidence than any progress has been made toward a resolution.
Note my comment on Bug 369862:

"The semantics of "Recent" are not very clear. Does it mean a folder that new
mail has been added to (by filters)? A folder that I've recently selected to
view messages in? A folder that I've moved or copied messages to/from?"




While almost a year old, this is still marked as a "New" issue. If you want this issue addressed you will need to vote for it. Currently there are 12 email addresses in the CC list but only 3 votes. Define, design and kvetch all you like but be sure to VOTE as well: https://bugzilla.mozilla.org/votes.cgi?action=show_user&bug_id=370324#vote_370324
I have been using QuickMenuMC which is why I almost forgot about this bug. But using an extension to do the work of what is supposed to be built-in shows how much it is important for me.

Item 2 of ggerard's original bug has been solved since (I am using 2.0.0.9).
Assignee: mscott → nobody
Flags: wanted-thunderbird3?
(In reply to comment #20)
> I have been using QuickMenuMC 

nice extension. However, for those of us who use trunk ...

I just had an interesting flash of activity which caused context menu's recent list to go bonkers - I recently unchecked the advanced option "show only subscribed folders" for one account, which then showed a few dozen of these folders in the recent list.


Does anyone sees ggerard's problem #1?  If not, the suggestions here need to go to Stephan's Bug 431256 – "Recent Folder" list would be much more useful in another way, or morph this bug to that description.
Keywords: helpwanted
would be nice to fix up for TB3

related: 
bug 351822 removed account stays in "Recent" folder view (until I switched views)
Bug 348401 -  [rfe] add a 'remove from recent' context menu item for folders in the folder pane
Component: Mail Window Front End → Folder and Message Lists
QA Contact: front-end → folders-message-lists
similar to folder pane bug 43125.
xref bug 491304 (show >15 recent folders)

(In reply to comment #11)
> Bear in mind that to fix this, you may have to use a different data source for
> the recent folder menu, since we want things like inboxes and folders that have
> messages filtered into them in the mru folder view in the folder pane.

David, Do we really need to meddle with the producer of the information/list of recent folders?  I'm wondering if it isn't just cleaner to filter unwanted folders out of the list when context menu is accessed

So folders that aren't likely to be a valid move target, or that have other means of moving messages directly to them, such as:
Archives
Templates
Drafts
Trash
Junk
Sent
RSS folders

target folders of filters would of course still be problematic
I believe when I wrote that, the menu item was populated by an rdf datasource. Now that it's not, we have a lot more flexibility, I would hope.
still wanted‑thunderbird3?

Our story starts with bug 350661 - Add Recent Folder Target to Move and Copy Menus

"Recent" folder move and copy are still mostly, i.e. majorly broken.  Itemized below is what fails within the usage of the existing information source as implemented by bug 350661, namely the list of recently *visited* folders. I tested several issues. Results vary between TB3 and TB2. To simplify things and because nothing will get fixed in TB2, my comments are based solely on tests of TB3.  This bug's focus remains on the original problem, despite most comments being related to comment 26. But might become a meta because there are so many issues.


1. is summarized by comment 26 which filters the list of folders, helps get us part way to bug 431256 (new bug pending)

Other issues are fundamental flaws

2. is bug 501076  List of "Recent" folders (for Move-to / Copy-to / ...) only updated after thunderbird restart

3. I sometimes see move's context menu is not the same as the copy's context menu (see screen shot coming up) (needs bug or fundamentally bug 501076?)

4. is "Message->Move->Recent" and Copy vary from context menu (needs bug or fundamentally bug 501076?)

5. (not tested) I suspect bug 351822 also affects context menu

6. is bug 498069/bug 431256. Doing bug 431256 probably eliminates the need for bug 498069. 431256="Recent Folder" list would be much more useful as triggered only by manual message move and copy MRU (most recently used). Which may go too far for some people's tastes

David's comment 27 gives me some hope that all but #6 can be fixed for TB3. I doubt the remaining time permits getting the "big fix" of #6.

bug query:
https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=allwordssubstr&short_desc=recent&product=MailNews+Core&product=Thunderbird&long_desc_type=anywordssubstr&long_desc=folder&bug_severity=major&bug_severity=normal&bug_severity=minor&bug_severity=trivial&bug_severity=enhancement&chfieldto=Now&field0-0-0=short_desc&type0-0-0=anywordssubstr&value0-0-0=menu+context+folder+wrong+command+messag&field0-1-0=short_desc&type0-1-0=nowordssubstr&value0-1-0=scroll+build+%5Crecent+address+retention+&field0-2-0=bug_id&type0-2-0=nowordssubstr&value0-2-0=+66004+202356+276227+278865+305340+320037+334415+
Severity: normal → major
Depends on: 501076
screen shot for #3 shows "move>recent" is out of date. copy>recent has most correct list of folders

#4 - it seems like Message>Move (and Copy) is also not always correct, but often it is correct when the context menu's recent is not

notes:
* some issues may be trunk regressions, i.e. don't originate with bug 350661
* tests based mostly on existing profile, results may differ when using a new profile. For example, I created a new imap account in a new profile and it flooded the recent list with tons of folders (more than the minimum display), much like I describe in comment 21. The list didn't reduce until I restarted, and then it showed only the inboxes for the imap account and local
Attachment #395936 - Attachment is obsolete: true
circled folders are missing from Move's list
Version: 2.0 → Trunk
(In reply to comment #17)

> ... Is there any documentation describing how it's *supposed* to work?
> That would be a useful starting place for (a) discussions about whether
> the design is optimal and (b) reporting discrepancies between expected
> and observed behavior.

I'm renewing this plea for documentation laying out how the "feature" is intended to behave.  I can't bring myself to believe that the development team randomly cobbles up modules for Thunderbird with no design or requirements documents, making it up as they go along.

> This problem certainly seems to be a serious annoyance to a lot of users; I'm
> dismayed to see it still flagged as New after almost a year, with no evidence
> than any progress has been made toward a resolution.


The comment I'm replying to is itself well over two years old.  How long does a bug need to age before it's no longer "new"?
URL: http://gsfn.us/t/bnmx
Whiteboard: [gs]
Where is this going? I guess nobody wants to take this project :(
Another data point for this "new" (filed in February 2007) bug: I just noticed that Thunderbird (3.1.7 on Ubuntu 10.10) added a folder in which no messages have *ever* been stored, either manually or by a rule, to the Move To -> Recent list.  The folder contains only other folders which themselves contain messages.  How can TB possibly have come to the conclusion that this is a folder in which messages have been stored recently when it has never happened, much less happened recently?  Again, where is the documentation for how the software is supposed to work?  Please!
It's getting worse.  Now the "Move to ... again" menu option is sometimes grayed out.  The ellipsis is correctly filled in with the name of the folder to which I most recently moved a message, so it knows what it needs to know in order to do what I want.  The TB team is making it harder to use the software instead of easier.  I have no idea whether this latest twist is part of the same bug, as whoever implemented this portion of the program has either been very adamant about refusing to share whatever design or functional documentation against which the implementation was performed, or did the implementation off the top of his or her head, without any such documentation.
Bob: that's unrelated, it was bug 574351 which should be fixed for tb 3.1.8 which will get released shortly.
this adds a separate timestamp set whenever we move/copy a message to a folder, and builds the move/copy recent sub-menu from that. This makes the menus much more useful for me, so asking Bryan for ui-review.

I noticed that the thread pane context menu recent-menu doesn't get added to during a session, unlike the toolbar message move/copy sub-menu.  It seems to be built the first time you use it, and then cached. I suspect that if this lands, that'll be the next thing people complain about, so I suspect I should fix that as well, if possible - Bryan, thoughts?
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
Attachment #521069 - Flags: ui-review?(clarkbw)
I take it back - neither menu gets rebuilt after the first showing.
(In reply to comment #40)
> I take it back - neither menu gets rebuilt after the first showing.

it's more nuanced than that I think. under what steps is it never rebuilt for you?
(In reply to comment #41)
> (In reply to comment #40)
> > I take it back - neither menu gets rebuilt after the first showing.
> 
> it's more nuanced than that I think. under what steps is it never rebuilt for
> you?

For example, open the context menu, move to, recent, and close the menu. Then, select a folder not in the recent list, go back to the original, and then bring up the same menu. The new folder should show up in the recent list; it doesn't. (This is without my patch - with my patch, you would try moving the message to a folder not in the list).
Comment on attachment 521069 [details] [diff] [review]
add a separate move/copy timestamp

(In reply to comment #39)
> Created attachment 521069 [details] [diff] [review]
> add a separate move/copy timestamp
> 
> this adds a separate timestamp set whenever we move/copy a message to a folder,
> and builds the move/copy recent sub-menu from that. This makes the menus much
> more useful for me, so asking Bryan for ui-review.

This looks good to me.

> I noticed that the thread pane context menu recent-menu doesn't get added to
> during a session, unlike the toolbar message move/copy sub-menu.  It seems to
> be built the first time you use it, and then cached. I suspect that if this
> lands, that'll be the next thing people complain about, so I suspect I should
> fix that as well, if possible - Bryan, thoughts?

Yeah, that makes a lot of sense for these to be consistent.
Attachment #521069 - Flags: ui-review?(clarkbw) → ui-review+
this makes the recent folders menus update when a folder has a message moved/copied to it, and it prevents the most recently moved timestamp from changing in the case of archive or non-user-initiated moves like message filters.
Attachment #521069 - Attachment is obsolete: true
Wayne, I'll try to generate a try server build, though our try servers don't seem to be turned on at the moment.
try server push worked this morning - if the builds succeed, they'll be here:

http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/bienvenu@nventure.com-9f2625c5e351
tested http://stage.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/bienvenu@nventure.com-278298336834/tryserver-win32

looking great!

- alpha order is always maintained
- list changes immediately
- copy and move have same list
- context and message menu have same list
- not affected by imap filter, and rss filter
- works between news, local and imap
- doesn't impact "recent folders" pane afaict
- list maintained across restart

some issues:
- folder names not transferred from old method to new ... probably can be ignored
- on first startup there is no information in the list, clicking "recent" gives the little "null" box - which is uninformative to the user. Probably not a new issue, and I'm not sure if there is a bug report on that.
- duplicate folder names don't have account name, for example inbox - different bug
(continuing after inadvertent save)

fixed
- Bug 498069 - Moves by Filter should not affect the list of folders in the "Message => Move To => Recent" menu
- Bug 431256 - "Recent Folder" list would be much more useful as triggered only by manual message move and copy MRU (most recently used)
- Bug 501076 - List of "Recent" folders (for Move-to / Copy-to / ...) only updated after thunderbird restart

correction
- duplicate folder names do have account name IF there is actually a duplicate AND it's a special folder. for example if there are two inboxes, the account name is shown but if only one, then no account name

don't know
- (didn't test) Bug 386385 - rss feeds and folders intermixed in "Unread", "Favorites" and "Recent" folder views

needs bug?
- deleted folder isn't removed from list. note: irrc there was a bug (fixed iirc) that dataloss could occur if folder removed and "move again" was used
thx very much for testing all this - all the results are expected - the empty menu would have been true for new profiles with the old code, except that folders you opened would have been in the list, e.g., the inbox. This won't affect bug 386385. Should have a bug on deleted folder not getting removed from the list (though a restart will remove it, or anything that does a rebuild, like moving a message to a folder, so this patch would help somewhat). I believe, but haven't tried, that prior to this patch, renaming a folder that was in the recent list would hang the app, because we would teardown and rebuild the menu, which removed and added a listener, which caused the new listener to get the rename notification, rinse, repeat forever.
If I may suggest: a sensible default for the empty list would be "Trash" and "Junk".
(In reply to comment #51)
> If I may suggest: a sensible default for the empty list would be "Trash" and
> "Junk".

That is a strange suggestion, given that a user will never see trash in the future - it is excluded from being listed.  delete a message, and move a message to trash, and you see it won't be listed in "recent", even though it does appear in the "move again" setting.  (I forget the complete list of exclusions, but junk is probably also one of them)
David, I am seeing a local folder in "recent" which is a target of an imap filter.  So there may be a hole in the "filter exclusion" - though perhaps not caused by your changes.
yes, junk and trash are specifically excluded from the recent list, so that would be a bit odd.

I'm surprised that imap to local folder filter move would make the local folder recent because allowUndo should be false in that case.
(In reply to comment #54) 
> I'm surprised that imap to local folder filter move would make the local folder
> recent because allowUndo should be false in that case.

hmm, now I am seeing several other filtered folders, including the only map folder that's filtered
(mentioned on IRC) ... filter folder is definitely an issue - one appeared in the recent list that I never enter/do anything with. But the appearance of filtered folders seems random. For example, one imap folder that gets lots of filtered mail, but never any manual moves or copies, isn't in the recent list.


Also, in working with Bug 517273 - Move, Copy, ... in standalone window when message opened from Search Messages - I'm seeing some errors in console like
 Error: this.folderDisplay.treeSelection is null
Source File: chrome://messenger/content/messageDisplay.js
Line: 556

Will investigate in more detail later.
also now see my two imap Trash folders in the list
David, At some point I stopped using the tryserver build because I wanted my add-ons back. If the patch is working for you, perhaps land it and get wider testing and I can revisit the issues previously cited.
wayne, I'm trying to write a mozmill test for it, which is proving time consuming...
Attached patch proposed fix with mozmill test (obsolete) — — Splinter Review 
this fixes a bug in the prev patch with local folders, which may or may not have been what Wayne was seeing with his testing...
Attachment #521408 - Attachment is obsolete: true
Attachment #522693 - Flags: review?(bugzilla)
Just installed the tryserver build on Linux.

Moved two messages, then I did not get the context menu when I right-clicked on a message ...

Well, after half a minute or so it worked again, and the "Recent" menu has two entries. But performance seems to be bad ...

This also happened after moving more message - and now it's still not working after several minutes.
How many folders do you have? Building up the recent menu involves iterating over all your folders (it always has), but we used to only ever do it once. Now we do it whenever you move a message to a folder. In normal profiles, it's fast, but if you have something goofy, such that panacea.dat doesn't exist/work, it can be slow.
I have 1083 IMAP folders, plus very few local folders (and some dozen newsgroups).

panacea.dat exists and is regularly modified. Size about 650k.
Jens, does your slow recent performance also happen with TB started in safe mode?
(In reply to comment #64)
> Jens, does your slow recent performance also happen with TB started in safe
> mode?

Yes.
Do I understand correctly(In reply to comment #39)
> Created attachment 521069 [details] [diff] [review]
> add a separate move/copy timestamp
> 
> this adds a separate timestamp set whenever we move/copy a message to a folder,
> and builds the move/copy recent sub-menu from that. 

I don't know how time-consuming it is to go through all my n thousand folders. But let's assume for now that _is_ what takes so long until I get a context menu again.

Wouldn't it be better / more efficient to store a list of x MRU folders, and when a folder gets moved or copied to, put it at the top of the list, and go through the list to see if it's already there (O(x) = O(1)) and if so, delete it?
(In reply to comment #66)
> 
> I don't know how time-consuming it is to go through all my n thousand folders.
> But let's assume for now that _is_ what takes so long until I get a context
> menu again.
> 
> Wouldn't it be better / more efficient to store a list of x MRU folders, and
> when a folder gets moved or copied to, put it at the top of the list, and go
> through the list to see if it's already there (O(x) = O(1)) and if so, delete
> it?
The menu is sorted alphabetically, and you have to kick out the oldest folder, and the menu already is the list of folders, but I agree that the menu could be updated instead of rebuilt when an MRM time stamp changes.
Jens, as far as I know, this patch hasn't landed, so any performance delay isn't a result of this patch.
(In reply to comment #68)
> Jens, as far as I know, this patch hasn't landed, so any performance delay
> isn't a result of this patch.

I of course used the tryserver build.
(In reply to comment #70)
> builds with latest patch will be here -
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/bienvenu@nventure.com-2537bd161c4b

I think this failed for linux ...

Anyway, I have the same problem as before (also in safe-mode).
(In reply to comment #71)
> (In reply to comment #70)
> > builds with latest patch will be here -
> > http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/bienvenu@nventure.com-2537bd161c4b
> 
> I think this failed for linux ...
> 
> Anyway, I have the same problem as before (also in safe-mode).

(I tried with linux 64 bit, which was available).

I now tried on Windows 7, on a completely different profile. And I again get this issue where the context menu is not displayed after moving a message using the context menu. After some minutes, it's there.
OK, I see the issue with the popup menu now - I'm not sure why I didn't see it earlier (though I think I did see it and fix it - the issue had to do with rebuilding the menu while it was still up, or something like that...)
Attached patch fix for issue Jens saw (obsolete) — — Splinter Review 
This makes us teardown and rebuild the menu on a timeout, so that we don't try to do so while the menu is up (move/copy can be synch, essentially, so we get an MRM time change notification while the menu is still open).
Attachment #522693 - Attachment is obsolete: true
Attachment #524736 - Flags: review?(neil)
Attachment #522693 - Flags: review?(bugzilla)
Keywords: helpwanted
Works fine.

I guess any polish, or making this more customizable, should go into new bugs?
(In reply to comment #76)
> Works fine.
> 
> I guess any polish, or making this more customizable, should go into new bugs?

yes, I think the patch as is is sufficient for the vast majority of users.
Attachment #524736 - Attachment mime type: application/octet-stream → text/plain
Attachment #524736 - Flags: review?(neil) → review?(neil)
ugh, thx, Ludo - autocomplete really failed me!
(In reply to comment #75)
> This makes us teardown and rebuild the menu on a timeout, so that we don't try
> to do so while the menu is up (move/copy can be synch, essentially, so we get
> an MRM time change notification while the menu is still open).
Of course the irony is that the folder was already in the list so no rebuild was necessary. I can't see an obvious way around this without switching to one of the other notifications that allows passing the old MRM time in, so that you can then determine that the folder was already in the list based on the oldest MRM time in the list (which you would have cached when building the list).

The other alternative idea I came up with would be to tweak _buildRecentMenu to work on an existing menupopup (created by _build) so that there would only be a slight flicker as it rebuilt the menuitems.
I started using the tryserver build #2 today, dated 2001-04-08, and based on the error console started experiencing what seems like bug 591722 / bug 606479. inbox only. happens in clusters, i.e. several deletes don't complete until I navigate away from inbox. then several deletes have no problem.  Perhaps it's purely coincidental and unrelated to using the tryserver build - but I've never seen this before.

xref bug 644601.
(In reply to comment #80)
> I started using the tryserver build #2 today, dated 2001-04-08, and based on
> the error console started experiencing what seems like bug 591722 / bug 606479.
> inbox only. happens in clusters, i.e. several deletes don't complete until I
> navigate away from inbox. then several deletes have no problem.  Perhaps it's
> purely coincidental and unrelated to using the tryserver build - but I've never
> seen this before.
> 
> xref bug 644601.

the try server build had a couple other patches in it, so it's extremely unlikely that the patch in this bug is responsible for any weirdness you've seen deleting messages.
previously I had been using 2011-03-21 so yeah, it's very possible I am impact by some other patch.
(In reply to comment #79)
> determine that the folder was already in the list
Hmm, I guess the menuitems already know their folder. Or you could cache the recentFolders array to see whether the notified folder is in the list.
(In reply to comment #79)

> Of course the irony is that the folder was already in the list so no rebuild
> was necessary.

The menu still needs to be rebuilt on a timeout, it turns out, even if you're not picking a recent folder. I.e., I took out the timeout part of rebuilding, and moved a message to a different folder, not in the recent list, and I ran into the problem Jens had.
Attached patch more wip — — Splinter Review 
checking if the folder already existed opened a whole can of worms (e.g., _getChildForItem was completely broken, which caused a lot of other issues to never be seen...).

Anyway, this mostly works, though there are errors where aThis._ensureInitialized doesn't exist, which I need to figure out.
Attached patch proposed fix (obsolete) — — Splinter Review 
this patch doesn't seem to cause any extra error cruft on the console...
Attachment #525907 - Flags: review?(neil)
Comment on attachment 525907 [details] [diff] [review]
proposed fix

>-          this._teardown();
>+          this._teardown(true);
Obsolete change?

>+        _clearMenu: function(aThis) {
>+          if (aThis._menu._teardown)
>+            aThis._menu._teardown();
[I can't decide whether this would be better with menu as the parameter]

>-          var child = this._getChildForItem(aItem);
>+          var child = this._getChildForItem(aItem, this._menu);
[Might be worth making the second parameter optional defaulting to _menu]

>-            this._menu._setCssSelectors(child, child._folder);
>+            this._menu._setCssSelectors(child._folder, child);
Oops, how long has this been wrong!
[On an unrelated note, I take it this fails if the same folder is in the recent submenu and in the main popup, because it only finds it once.]

>+            if (this._menu.getAttribute("showRecent") != "true" ||
>+                !this._menu.firstChild || !this._menu.firstChild.firstChild)
>               return;
>+            var recentMenu = this._menu.firstChild.firstChild;
>+            if (!recentMenu || this._getChildForItem(aFolder, recentMenu))
Nit: you just checked that this._menu.firstChild.firstChild is null above (although I'm not sure which test should be removed).

>+        nsCOMPtr<nsIAtom> MRMTimeChangedAtom = MsgGetAtom("MRMTimeChanged");
>+        SetMRMTime();
>+        NotifyFolderEvent(MRMTimeChangedAtom);;
Nit: double ; also odd ordering of lines (would not put SetMRMTime in middle).
folderWidgets.xml has been broken since we moved to Mercurial - I didn't look further back than that.
Attachment #525907 - Attachment is obsolete: true
Attachment #526508 - Flags: review?(neil)
Attachment #525907 - Flags: review?(neil)
Comment on attachment 526508 [details] [diff] [review]
fix addressing comments
[Checked in: Comment 92 & 105]

Hmm, well SeaMonkey's still using RDF for this stuff, which must explain why we don't have the problem ;-) But it all compiles, and the code looks good.
Attachment #526508 - Flags: review?(neil) → review+
Comment on attachment 524736 [details] [diff] [review]
fix for issue Jens saw

Removing what I assume is an obsolete review request and obsoleting the patch.
Attachment #524736 - Attachment is obsolete: true
Attachment #524736 - Attachment is patch: true
Attachment #524736 - Flags: review?(neil)
fixed on trunk - http://hg.mozilla.org/comm-central/rev/958e3615a3b8
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
(In reply to comment #92)
> fixed on trunk - http://hg.mozilla.org/comm-central/rev/958e3615a3b8

Note that Thunderbird's latest-comm-central is currently softlinked to latest-comm-2.0 -- unless I'm mistaken, to have any meaning for current Thunderbird builds, this fix should be ported to the comm-2.0 repository.
status-thunderbird5.0: --- → ?
I thought that repo was for SM 2.1 - Tinderbox for Miramar still refers to comm-central trunk and mozilla-aurora repo's.
In reply to comment #94
Ah. Then "at least one of us" (probably me) is misinformed or misunderstood what he was told: IIRC Ludovic "Usul" Hirlimann told me on IRC a couple of days ago that Thunderbird "trunk" builds are actually from the 2.0 branch.
status-thunderbird5.0: ? → ---
Component: Folder and Message Lists → Backend
Flags: wanted-thunderbird3?
Product: Thunderbird → MailNews Core
QA Contact: folders-message-lists → backend
(In reply to comment #95)
> In reply to comment #94
> Ah. Then "at least one of us" (probably me) is misinformed or misunderstood
> what he was told: IIRC Ludovic "Usul" Hirlimann told me on IRC a couple of days
> ago that Thunderbird "trunk" builds are actually from the 2.0 branch.

He meant the mozilla 2.0 branch, I believe, and we've now switched to mozilla-aurora.
(In reply to comment #93)
> (In reply to comment #92)
> > fixed on trunk - http://hg.mozilla.org/comm-central/rev/958e3615a3b8
> 
> Note that Thunderbird's latest-comm-central is currently softlinked to
> latest-comm-2.0 -- unless I'm mistaken, to have any meaning for current
> Thunderbird builds, this fix should be ported to the comm-2.0 repository.

Thunderbird no longer has dealings with comm-2.0 (that'll be a SeaMonkey
specific repo). If you want things for SeaMonkey then you need to either file a
SeaMonkey specific bug, or request an appropriate SeaMonkey flag.

As it is the changes in the patch affected Mailnews Core, so I've moved this
bug there (where there are SM flags). You may also want to actually check there is an issue in SeaMonkey before requesting flags - especially considering comment 90.
(In reply to comment #97)
> (In reply to comment #93)
> > (In reply to comment #92)
> > > fixed on trunk - http://hg.mozilla.org/comm-central/rev/958e3615a3b8
> > 
> > Note that Thunderbird's latest-comm-central is currently softlinked to
> > latest-comm-2.0 -- unless I'm mistaken, to have any meaning for current
> > Thunderbird builds, this fix should be ported to the comm-2.0 repository.
> 
> Thunderbird no longer has dealings with comm-2.0 (that'll be a SeaMonkey
> specific repo). If you want things for SeaMonkey then you need to either file a
> SeaMonkey specific bug, or request an appropriate SeaMonkey flag.

Ok, so I missed the fact that you wanted it for Thunderbird. At the moment Thunderbird is still coming out of comm-central, when it changes, it'll be announced and there will be specific approval flags available on patches.
this test seems to be failing on Tinderbox sometimes - it would fail if some of the previous tests moved messages to other folders, and the profile was re-used for the current test, though my push to try didn't show the issue...I'll look into it.
In reply to comment #98:
Ah, so I played wiseass while not knowing what I spoke about. Sorry for the bugspam since comment #93.
(In reply to comment #97)
> You may also want to actually check there
> is an issue in SeaMonkey before requesting flags - especially considering
> comment 90.

"status-seamonkey2.1=?":
Fwiw, my rule would be to sync' unless it breaks something. (Even if it's not fixing anything as is.)
Nonetheless, SM 2.1 is supposed to be very near release, so we might just (already) "stop" sync'ing on that (brand new) branch.
Callek, ftr, this is the one changeset that didn't make comm-2.0 before I fixed bug 650852.
status-seamonkey2.1: --- → ?
this changes the test not to assume that there are no recent folders when the test is run, and to create unique folder names.

I think this will reduce the tinderbox errors, but I'm not convinced it will fix them entirely, since I can't reproduce the issue, and the test failures aren't entirely consistent with their being existing menu items.
Attachment #526840 - Flags: review?(bugzilla)
Comment on attachment 526840 [details] [diff] [review]
make menu counts relative
[Checked in: Comment 104 & 106]

I'm happy to try this. If it doesn't work, we could try moving the test to its own standalone directory.
Attachment #526840 - Flags: review?(bugzilla) → review+
pushed http://hg.mozilla.org/comm-central/rev/3d150a8c87ee to see how it affects mozmill failures.
Comment on attachment 526508 [details] [diff] [review]
fix addressing comments
[Checked in: Comment 92 & 105]

http://hg.mozilla.org/releases/comm-2.0/rev/958e3615a3b8
Attachment #526508 - Attachment description: fix addressing comments → fix addressing comments [Checked in: Comment 92 & 105]
Comment on attachment 526840 [details] [diff] [review]
make menu counts relative
[Checked in: Comment 104 & 106]

http://hg.mozilla.org/releases/comm-2.0/rev/3d150a8c87ee
Attachment #526840 - Attachment description: make menu counts relative → make menu counts relative [Checked in: Comment 104 & 106]
I haven't seen any more of these failures on mozmill (though I haven't done an exhaustive look)
I'm running 6.0 on Mac and the problem seems to be fixed.

Thanks!
Blocks: 551137
Blocks: 1332488
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: