261199 - UI for defining and editing virtual folders

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[David :Bienvenu]

We need a UI for defining (File | New Virtual/Search Folder) and editing (folder
properties) virtual folders. I believe the same dialog can be used for both.

The dialog needs to allow the users to define/edit the search terms, and the
search scope (the folders to be searched over). I think to be really useful, we
should allow the users to pick an arbitrary set of folders to search over,
instead of a folder and optionally its subfolders. I think something like the
subscribe UI where the user can check/uncheck each folder would be good. 

It would be nice if this dialog also allowed the user to specify that the search
be done offline for imap and news folders, since searching a lot of imap folders
can be prohibitive on servers that don't index, like UW in its default
configuration. I have not implemented the backend for this but it should be trivial.

Right now, I don't know if we do pan-server searches in the backend, but I think
we might - it could just be a matter of adding new scopes, and the backend
should just chain them. But, I could be wrong. It would be great if we could
have pan-server virtual folders. Maybe later...

The problem with the subscribe UI approach is that we couldn't use RDF like the
current subscribe UI's, since the "search over" attribute isn't folder specific,
but depends on what the virtual folder itself is.

CreateVirtualFolder in commandglue.js shows you how to create a virtual folder
(basically the search terms and folder uris are stored in the dbfolderinfo of
the db... and accountManager.saveVirtualFolders() saves all that info in
virtualfolders.dat - if we could add a "seach offline boolean, we'd add that
there - I can do that, actually...)

Comment 1

[Scott MacGregor]

sairuh was asking about this bug the other day

Comment 2

[Scott MacGregor]

Attachment: work in progress snapshot

dialog for creating a new virtual folder. Not finished yet.

Still to do:

1) add a sub dialog for choosing the folders to search
2) add support for editing an existing virtual folder
3) add a check box for searching online or offline attributes
4) polish

Comment 3

[Scott MacGregor]

Attachment: more work...hook up the ability to edit an existing VF

Comment 4

[Scott MacGregor]

Attachment: update

Progress update. Getting real close now. Everything is pretty functional. Just
down to the polish stage.

I haven't added some of the more advance feature requests david described like
select all sub folders and search online or offline yet.

Comment 5

[Tuukka Tolvanen (sp3000)]

> select all sub folders and search online or offline yet.

With all this talk about selecting subfolders, I worry that recursive searches
in a folder where new subfolders are added wouldn't apply to those new
subfolders. Say it ain't so :)

Comment 6

[David :Bienvenu]

I'm sorry to say that it is so - If we had some way of expressing a uri for all
sub-folders (e.g., mailbox://foo@bar/Inbox/*) then maybe we could do this. We
might make a special case for the account wide search, so if the search uri is
for an account, then we search all folders on the account...

Comment 7

[Scott MacGregor]

Attachment: just the back end RDF data source changes

breaking the patch down into pieces for an easier landing.

Comment 8

[Scott MacGregor]

Comment on attachment 162168 [details] [diff] [review]
just the back end RDF data source changes

bienvenu super reviewed this portion this morning for me.

Comment 9

[Scott MacGregor]

the RDF data source changes have landed on the branch and the trunk.

Comment 10

[Scott MacGregor]

Attachment: patch for thunderbird ready for review

Comment 11

[Scott MacGregor]

Comment on attachment 162268 [details] [diff] [review]
patch for thunderbird ready for review

This makes the new UI work for Thunderbird. I haven't ported it to seamonkey
yet.

Not everything is finished yet, most noticeably the check box for toggling
between offline and online searching and a check box in the folder list dialog
for including subfolders (but you can manually choose as many folders as you
want).

I also haven't converted advanced search and the Views menu to use the new
dialog. Quick search has been converted to bring up a pre-filled version of the
new dialog.

Comment 12

[Scott MacGregor]

Attachment: screen shot of the new UI elements

Comment 13

[Scott MacGregor]

QA: Here is a summary of the new behavior

1) File / New / Saved Search Folder --> brings up new virtual folder UI

2) right clicking on an existing virtual folder, choose properties --> should
see virtual folder UI showing current VF properties. Title of the dialog should
reflect the name of the folder

3) Quick Search / Save As Search Folder --> should bring up new virtual foldere
UI with fields pre-filled for name, folder to create under, folder to search and
search terms.

4) If you attempt to create a new virtual folder and you don't choose any
folders to search, you should get an error dialog. 

5) Verbage has changed from Virtual Folders to Saved Search Folders in the UI

6) canceling the edit virtual properties dialog should not change any properties

7) canceling creationg of a new virtual folder should not do anything

8) After modifying the properties of an existing virtual folder, when you hit
ok, if that folder is the currenly loaded folder we should reload it, reflecting
the changes you made to the search criteria.

Known Issues still under development for this bug
--------------------------------------------------
1) Views / Save as Search Folder and Advanced Search / Save as Virtual Folder
need to use the new dialog and need language change for advanced search.

2) If you try to create a new virtual folder with a name that is already a valid
folder then we should bring up an error dialog.

3) The folders in the new folder picker dialog are not sorted correctly with the
special folders at the top like they are in the folder pane. I don't know why
yet. They are sorted alphabetically.

4) Need a checkbox in the folder picker dialog that says search sub folders.
Clicking on a folder would automatically select its subfolders for the virtual
folder search as well.

5) a checkbox in the main virtual folder dialog for doing an offline search
instead of searching online.

Those are most of the behavioral issues I can think of right now to pay
attention to when testing the new UI.

Comment 14

[David :Bienvenu]

Comment on attachment 162268 [details] [diff] [review]
patch for thunderbird ready for review

thx so much for doing this! sr=bienvenu, with some nits:

     if (isServer || isInbox)
-      SetMenuItemLabel("menu_NewFolder",
-	 gMessengerBundle.getString("newFolder"));
+      SetMenuItemLabel("menu_NewFolder",
gMessengerBundle.getString("newFolder"));
     else
-      SetMenuItemLabel("menu_NewFolder",
-	 gMessengerBundle.getString("newSubfolder"));
+      SetMenuItemLabel("menu_NewFolder",
gMessengerBundle.getString("newSubfolder"));

this could just be:

SetMenuItemLabel("menu_newFolder", gMessengerBundle.getString((isServer ||
isInbox) ? "newFolder" : "newSubfolder");


Should this commented out code just be removed? :
+
+//  if (window.arguments[0].preselectedURI) {
+//    var uri = window.arguments[0].preselectedURI;
+//    var folder = GetMsgFolderFromUri(uri, true);
+//  }


+function onCancel()
+{
+  // generateFoldersToSearchList will undo all of our folder property changes
for us
+  generateFoldersToSearchList(); // ignore return string 
+  return;

you don't need the return;, right?

this is a little confusing. It seems like it could be written as one if,
assuming isContainerOpen doesn't have some side effect we're counting on: -
maybe 

if (obj.value != "twisty" && col.value == "selectedColumn")
  ReverseStateFromNode(row.value)


+    if (obj.value == "twisty") {
+	 if (gFolderPickerTree.view.isContainerOpen(row.value)) {
+	 }
+    }
+    else {
+      // if the user single clicks on the search folder check box, we handle
it here
+      if (col.value == "selectedColumn")
+	 ReverseStateFromNode(row.value);

here's a trick I learned from Brendan, I think:

+    if (arguments.searchTermSession)
+      gSearchTermSession = arguments.searchTermSession;
+    else
+      gSearchTermSession =
Components.classes[searchSessionContractID].createInstance(Components.interface
s.nsIMsgSearchSession);

can just be:

gSearchTermSession = arguments.searchTermSession ||
Components.classes[searchSessionContractID].createInstance(Components.interface
s.nsIMsgSearchSession);

in +function onOK()

I don't think you want +    msgDatabase.summaryValid = true;

since you're editing an existing folder...

Comment 15

[Scott MacGregor]

Attachment: patch to make views and advanced search use new Virtual Folder UI

This patch converts the advanced search / Save as Virtual Folder button
and the
Views / Save As Search Folder menu item

to use the new virtual folder dialog UI

Unfortunately this can't be checked into the trunk until I back port the new UI
to seamonkey since it's changing shared code.

But this patch and this behavior has now been checked into the aviary 1.0
branch for Thunderbird.

Comment 16

[Scott MacGregor]

Attachment: check to see if a folder exists with the provided name and alert the user if so

If you try to create a new virtual folder with a folder name that already
exists at the same level in your folder hierarchy as the new virtual folder
then alert the user and don't create the virtual folder.

Comment 17

[Scott MacGregor]

Comment on attachment 162441 [details] [diff] [review]
check to see if a folder exists with the provided name and alert the user if so

David is nsIMsgFolder.containsChildName the right way to check if a folder
exists with the name the user provided? Do I need to do anything like escape
the user folder name or lower case it before calling this? It doesn't look like
it.

Comment 18

[David :Bienvenu]

Comment on attachment 162441 [details] [diff] [review]
check to see if a folder exists with the provided name and alert the user if so

containsChildNamed would mostly work, but it does a case-insensitive
comparison, which isn't quite right, especially for IMAP (though it errs on the
side of caution, so it's OK). I don't think you need to escape the name since
it's unicode, and we're comparing it with the unicode display name.

GetChildWithURI allows you to say whether you want a case-sensitive comparison
or not. If you pass in False for deep, it should do what you want (assuming you
can create the child uri at the point you want to call this...)

Comment 19

[Thomas Swan]

I just glanced at the screen shots for the UI elements.  There might be some
confusion as to what is a real folder and what is a virtual (saved search)
folder.  A magnifying glass or some other slightly different icon for the
virtual folder would help avoid any unnecessary confusion.

Comment 20

[Scott MacGregor]

Attachment: CSS for the folder list picker dialog for Mac OS X

add CSS rules for the dialog that lets you pick which folders to search for OS
X.

Comment 21

[sairuh (rarely reading bugmail)]

re comment #19: that's covered by bug 263989.

Comment 22

[Scott MacGregor]

Attachment: modify folder list picker dialog to only let you choose folders from the same account

Who says I can't write code while watching the Red Sox....

Comment 23

[Scott MacGregor]

Comment on attachment 162533 [details] [diff] [review]
modify folder list picker dialog to only let you choose folders from the same account

David, this patch adds an account combo box at the top of the folder picker
dialog. You have to choose an account and then choose the folders to search on
that account. This prevents you from selecting folders across many accounts.

Comment 24

[Scott MacGregor]

Attachment: new folder picker with account combo box screen shot

Comment 25

[David :Bienvenu]

Comment on attachment 162533 [details] [diff] [review]
modify folder list picker dialog to only let you choose folders from the same account

who says I can't review code while watching the red sox with my eyes closed?
:-)

Comment 26

[Scott MacGregor]

Please note that 265014 was just fixed which allows you to search online or
offline for online servers. Faster results if you search offline but you may get
stale data for imap and news servers.

Comment 27

[Scott MacGregor]

FYI, there are two remaining issues keeping this bug open for branch and trunk:

1) The folders in the new folder picker dialog are not sorted correctly with the
special folders at the top like they are in the folder pane. I don't know why
yet. They are sorted alphabetically.

2) Need a checkbox in the folder picker dialog that says search sub folders.
Clicking on a folder would automatically select its subfolders for the virtual
folder search as well.

I may punt on issue #2 for now but would still like to fix issue #1.

Comment 28

[Joe McCabe]

I may be overlooking something obvious, but should this be working on SeaMonkey
trunk right now, or is it checked in yet dormant?

I see this was checked in on 2004-10-18 20:08, but my 2004102013 build shows no
evidence of File | New | Saved Search Folder under Messenger.

Comment 29

[Thomas Stache]

I have two additional nagging things with the folder picker dialog (using Build
ID=2004102022):

1. The account combobox *appears* empty while the selection of an account still
works surprisingly (also reported here:
http://forums.mozillazine.org/viewtopic.php?p=886915#886915 , but I don't see
the last account name here).

2. A "Select all folders" button that places the checkmark to all folders. My
ISP's IMAP server allows no hierarchy, so the "search subfolders" wouldn't help.

Comment 30

[Marcia Knous [:marcia]]

Thomas: Regarding Item #1, this is a known issue that Scott is working on. Stay
tuned.

(In reply to comment #29)
> I have two additional nagging things with the folder picker dialog (using Build
> ID=2004102022):
> 
> 1. The account combobox *appears* empty while the selection of an account still
> works surprisingly (also reported here:
> http://forums.mozillazine.org/viewtopic.php?p=886915#886915 , but I don't see
> the last account name here).
> 
> 2. A "Select all folders" button that places the checkmark to all folders. My
> ISP's IMAP server allows no hierarchy, so the "search subfolders" wouldn't help.

Comment 31

[Scott MacGregor]

Attachment: RDF magic

RDF magic patch (don't claim to understand it) that fixed the last two big UI
nits:

1) Folders are now sorted properly in the folder picker dialog
2) The account picker in the folder picker dialog now shows the account names
in the drop down instead of using a height of 0 pixels for each row in the menu
list.

Comment 32

[Scott MacGregor]

Closing out this bug now that those last two issues have been addressed. If
there are any new problems with the new Thunderbird UI, we'll file new bugs now.
Thunderbird and saved search folders woo hooo!

Comment 33

[Cameron Simpson]

(In reply to comment #6)
> I'm sorry to say that it is so - If we had some way of expressing a uri for all
> sub-folders (e.g., mailbox://foo@bar/Inbox/*) then maybe we could do this. We
> might make a special case for the account wide search, so if the search uri is
> for an account, then we search all folders on the account...

Well you could adopt a glob idiom used in various other places: foo/* means all
immediate subfolders of foo, and foo/** means subfolders at any depth.

And later you might allow: foo/**/New or foo/**/!deleted etc.

Just a syntax idea FYI.

Comment 34

[Serge Gautherie (:sgautherie)]

Attachment: (Mv1) <widgetglue.js> [Checked in: Comment 36]

[Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE)

1 format + 1 reorder + 1 fix.

Could you (super-)review/check in this patch ? Thanks.

Comment 35

[Serge Gautherie (:sgautherie)]

Comment on attachment 177006 [details] [diff] [review]
(Mv1) <widgetglue.js>
[Checked in: Comment 36]

'approval1.1a2=?':
Trivial JS code cleanup, no risk.

Comment 36

[Serge Gautherie (:sgautherie)]

Comment on attachment 177006 [details] [diff] [review]
(Mv1) <widgetglue.js>
[Checked in: Comment 36]


Check in: { 2005-06-14 18:21	timeless%mozdev.org	mozilla/ mail/ base/
content/ widgetglue.js	   1.14 }

Comment 37

[Mike Cowperthwaite]

Comment on attachment 177006 [details] [diff] [review]
(Mv1) <widgetglue.js>
[Checked in: Comment 36]

patch is *not* "obsolete"