490326 - Smart folders - individual saved searches disappear

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect, P2)

Description

[Alex Ihrig [:Thunderbird_Mail_DE]]

In Smart folders it's possible to create individual saved searches. After creating the saved search, I changed the folder pane mode to "All folders" and back again to Smart folders. The before created saved search has disappeared.

IMHO we either should disable the possibility to create individual saved searches or they must be persistent.

Comment 1

[David Ascher (:davida)]

Making sure bienvenu sees this.

Comment 2

[David Ascher (:davida)]

I'm not sure how to repro this.  Can someone post clear STR?

Comment 3

[David :Bienvenu]

I'm also not sure this will still happen if we get rid of the top level smart folders account (though I suppose the saved search could have a smart folder as a parent).

Comment 4

[Magnus Melin [:mkmelin]]

STR: in smart folder mode:
 - right click the folder pane: Search
 - match something
 - Save as Saved Search
 - Create as subfolder of Smart Folders

Then, switch between folder modes. The saved search is gone.

Comment 5

[David :Bienvenu]

p2 (but those steps don't work for me - I can't pick the smart folders account as a parent)

Comment 6

[Magnus Melin [:mkmelin]]

It's not pickable in the list (if you pull it out) but it's filled in as default.

Comment 7

[Kent James (:rkent)]

I've looked at this as something simple that I could do to reduce the blocker backlog. One question though: would it be OK if I used one of the spare nsMsgFolderFlags to set a flag "Smart" on the smartfolder root and its immediate children? The flag would make it easier to recognize these folders - though a folder database property could also work. 

In addition to this issue of allowing saved searches, there is also the question of other parts of the context menu that should also be cleaned up. For example, you probably don't want to allow editing of the search attributes of the Smart subfolders either.

I propose that we disallow search for the smart root folder and its immediate children (Inbox, Drafts, etc.) to resolve the main symptom of this bug.

Comment 8

[Kent James (:rkent)]

Re 'would it be OK if I used one of the spare nsMsgFolderFlags to set a flag "Smart" on the smartfolder root and its immediate children?'

Never mind, I see that a property atom is already used for this purpose.

Comment 9

[Kent James (:rkent)]

Attachment: disable search for smart folders

This represents one possible approach to this, namely to disable search for smart folders. This is a little outside of my normal areas of expertise, so if I am really off base, please don't hesitate to tell me.

As I got into this, I see a variety of other little issues. For example, Trash context menus are very different then the other smart folders. I suspect Trash exists by default, and therefore cannot be named as a virtual folder.

It might be better to try to define a server type for "Smart Folders" that has proper properties (such as cannot search) and use that rather then folder properties to define behaviour. Or perhaps it would be better to allow the search, and just disable the saving of the saved searches.

Comment 10

[David :Bienvenu]

smart folders Trash has an issue which I've been investigating, and which I'm spinning off a new bug for. 

Again, the smart folders root item isn't going to be in the folder pane as soon as my fix for bug 490691 lands.

Comment 11

[Kent James (:rkent)]

It's possible then that this patch may not be needed at all in the long run. Virtual folders normally do not allow search in context menus, so if all of the smart folders are virtual, then in theory they should not allow search.

Comment 12

[David :Bienvenu]

yes, I don't think this patch should be needed - sorry you had to spend your time on this - I've been blocked on reviews for a while, so you can probably identify ;-)

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 376976 [details] [diff] [review]
disable search for smart folders

Cancelling review since it sounds this won't be needed. Please re-request if it does.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Adding qawanted and pushing to 3.0b4.  The desired QA help would be to test this bug and see if it's possible to reproduce on current nightlies, given that bug 490691 is likely to have made this problem go away.

Comment 15

[Wayne Mery (:wsmwk)]

perhaps Joe or Jay can comment.

I don't see any of the described behavior, including comment 6, using 20090601 build.

Comment 16

[Joe Sabash [:JoeS1]]

Saved searches seem to work fine for me.

Comment 17

[Alex Ihrig [:Thunderbird_Mail_DE]]

One thing is still possible in nightly build 20090601:

In Smart folders select a toplevel smart folder ('Inbox', 'Drafts', ...). Creating saved searches from the context menu isn't possible, but it _is_ possible from file menu > new > saved search. Doing this creates the saved search and the corresponding files in profile (e.g. 'Inbox.sbd' > 'Test_Saved_Search' and > 'Test_Saved_Search.msf'). Switching folder pane mode removes the saved search in the tree, but the files still exist in profile.

Maybe we could hide the menu item in 'Files' > 'New' > 'Saved Search', if a top level Smart folder is selected.

Comment 18

[Alex Ihrig [:Thunderbird_Mail_DE]]

(In reply to comment #17)
> search and the corresponding files in profile (e.g. 'Inbox.sbd' >
> 'Test_Saved_Search' and > 'Test_Saved_Search.msf'). Switching folder pane mode
> removes the saved search in the tree, but the files still exist in profile.

Removing the obsolete files from the profile (maybe nothing a normal user would do) leads to a crash immediately after Thunderbirds start (when folder tree is created). This occurs because of the left-over in virtualFolders.dat. Removing the left-over lines out of virtualFolders.dat solves the crash.

Comment 19

[David :Bienvenu]

taking to investigate Alexander's last two comments.

Comment 20

[David :Bienvenu]

File | new | saved search is only present for the smart inbox, not the smart drafts/templates/trash, which is odd.

Comment 21

[David :Bienvenu]

Attachment: disable new folder, saved search on smart folders

disables new saved search and new folder for the smart folders inbox.

Comment 22

[Magnus Melin [:mkmelin]]

Comment on attachment 391520 [details] [diff] [review]
disable new folder, saved search on smart folders

Sure, r=mkmelin with the context menu fixed too - http://mxr.mozilla.org/comm-central/source/mail/base/content/mailContextMenus.js#546

Comment 23

[David :Bienvenu]

Attachment: patch to land

ok, thx, this is what I'll land, then.

Comment 24

[David :Bienvenu]

fix checked in.