107598 - Filter UI: newly created folder not selected in filter action(IMAP) - Reopened for prefill case

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect, P2)

Description

[laurel]

Using oct29 commercial trunk

When creating a new folder through the filter rules dialog, it used to be (in
0.9.4 branch) that the newly created folder was displayed as the selected
destination folder in the filter action (when action set to Move to Folder). 
Now the selection doesn't change to reflect the new folder.

1.  Edit|Message Filters, click New to open new filter rules dialog.
2.  (Filter action is by default set to Move to Folder.)  Click New Folder,
select a parent and create a new folder.

Result:  After successful folder creation, filter action dropdown still shows
default (account level) as selection. The user must select the new folder from
the dropdown (adds an extra step).

Expected:  Filter action dropdown should change to show the new folder as the
selection.

Comment 1

[Navin Gupta]

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

Comment 2

[Navin Gupta]

It works fine for pop3. For imap, the folder is not immediately created , it takes
a while to create the folder on the server and when the server notifies us then
we create it locally. That is why it doesn't show up in the picker. 

Comment 3

[Michael Möllney]

1. Creating folders is only possible on the first stage of the folder hierarchy
(IMAP) while selecting a folder is possibple throughout the whole hierarchy..

2. In Filter-Panel, folder list is not updated (build 2001112013). First you
have to leave filter-creation-panel, then the new folder appears in the
folder-list in mail-window, then starting filter-creation again shows the new
folder in the move-to-list. Are you sure the filter-creation-panel is listening
to the imap-server and the imap-server sends a message to all connection
clients, that a new folder was created by itself? (I'm no expert in imap, but
perhaps you have to query the IMAP-Server if there are new folders?)

Comment 4

[Navin Gupta]

IMAP folders are not created immediately so it does not stick to the picker. 
I think we can just have this as release note. 

Comment 5

[laurel]

4.x filter ui is able to default to the new (IMAP) folder.

Comment 6

[Navin Gupta]

we should fix this one in mach V. 

Comment 7

[Navin Gupta]

Attachment: proposed fix

The fix is to createClient subfolder on the ui thread so that we can
immediately
set it on the picker. It doesn't hurt us because chances of folder creation are

pretty high and also we take care of duplication problem before sending command

to the server (this also fixes bug 45041). If folder creation failed on server
for some reason we delete the client folder and if it succeeds we set the flag
correctly in PossibleImapMailbox.

Comment 8

[Navin Gupta]

cc bienvenu for review

Comment 9

[Navin Gupta]

Also I cleaned duplicate error msgs code and added one duplicate msg to base. 

Comment 10

[David :Bienvenu]

I don't see us freeing this:

+          char* uriStr = ToNewCString(uri);

As you can imagine, I'm not crazy about this approach of assuming success and
trying to clean up in the case of failure. "chances of folder creation are
pretty high" scares me a bit, as does the fact that we might be ignoring the
whole mod-utf7 encoding issue both when creating the folder, and handling the
case where the folder creation fails. It could all work fine, but it would
require lots of testing, and I'm not sure there isn't some little hack that
could fix the filter ui without changing the imap folder creation backend.

The main things to test are folders where the online name and the folder name
end up being different, i.e., the folders that require mod-utf7 encoding. The
simplest case is folders with '&''s in them, although any non 7-bit ascii
character will work as well. If you try to create a folder with a name like
that, can we still load the folder correctly? If the creation of such a folder
fails, do we find the right sub-folder name in the oncreatefailed method? Let me
know if you need more info about mod utf7 encoding and what it means.

Comment 11

[Navin Gupta]

Good point, I forgot about modutf7.
Actually it could be done in this way but the problem becomes real complicated
with other servers hierarchyDelimiter and '/'. we just keep changing 
back and forth !!!

I have thought about another way to fix it by notifying FE that folder has been
created. 

Comment 12

[Navin Gupta]

Attachment: proposed fix

Add folderCreateFailed/Completed event that will tell us when imap folder
creation
is done and so we can set the name on the picker. We need to add folderListener

to filterEditor and I copied SetBusyCursor from mail3Pane js code because i
didn't
want to include entire js script file. 

patch also includes alert clean-up changes.

Comment 13

[David :Bienvenu]

Comment on attachment 69163 [details] [diff] [review]
proposed fix

r OR (not AND) sr = bienvenu, whichever one you need more. Seems like
marginally more trouble than the original approach, but seems like the right
thing to do.

Comment 14

[racham]

Comment on attachment 69163 [details] [diff] [review]
proposed fix

I don't think you need the following in SetBusyCursor() as they are not
relevant here.

+	var numFrames = window.frames.length;
+	for(var i = 0; i < numFrames; i++)
+		SetBusyCursor(window.frames[i], enable);

Remove those lines and try your patch again.

r=bhuvan

Comment 15

[Navin Gupta]

fix checked in w/ bhuvan's suggestion

Comment 16

[laurel]

Using feb22 commercial trunk build:
OK when using new folder in filter ui launched via Edit|Message Filters.
Not OK when using new folder in filter rules launched via prefill/"create
filters"  (prefill case does indeed work for pop/local folders).

Reopening for prefill/"create filters" case, appended to summary to reflect
current issue.

Comment 17

[Navin Gupta]

I am also seeing it for "Create Filter.. " case. 

Comment 18

[laurel]

Navin talked to me and decided to close this report and open a separate one for
the create filter/prefill case...see bug 127370.

marking this one verified

Comment 19

[laurel]

.