67606 - Folder properties dialog should allow renaming

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Jesse Ruderman]

The properties dialog for mail folders always has "name" as a read-only field.  
The field should be editable if the folder can be renamed.  (Perhaps 
the "rename" dialog should be removed in favor of allowing users to rename the 
folder using the Properties dialog.)

I'm not sure if the current architecture "knows" whether a folder can be 
renamed... see bug 67605 and bug 54446.

Comment 1

[Håkan Waara]

This is known, and I made the patch. The reason was that it should work as a
workaround for the bug where the field was enabled when one did "Folder
Properties.." on a newsgroup/server etc.

Comment 2

[James Green]

If we can tell if the folder is rename-able and we have a folder rename
function, this is doable, right?

Comment 3

[James Green]

From talking to hwaara: nsIMsgFolder contains a canRename property that we can
check to make the field readonly or not.

Now, when we click on the dialog's ok button, we will need to compare the
current folder's name with the name in the textfield, and if they're different,
try to rename. We should probably check for canRename first to avoid unecessary
comparison.

Once this has been implemented, should we hook any menu items for folder rename
to open the properties dialog instead? Then we can remove the rename dialog
completely.

Comment 4

[James Green]

This is basically working in my tree, except that I cannot call RenameFolder()
without problems. I've tried three methods:

1. including widgetglue.js and msg3PaneWindow.js and calling RenameFolder()
would work, but it has to get a reference to the messenger object, and that
works based on the |window| object. This is fine when called from the rename
dialog, but when called from the properties dialog, |window| is different and
the messager call fails.

2. Working around the above problem I tried calling
window.opener.RenameFolder(). This worked great, but the link to the folder was
not updates, and lots of exceptions spewed into the console, and the folder
couldn't be selected from the UI until after a restart.

3. I then tried to copy the messager stuff from RenameFolder into the
folderProps.js file and call it directly. This also doesn't work as I need to
get the messager object, and I'm not sure if that would be frowned upon as
adding bloat.

Suggestions requested.

Comment 5

[James Green]

I've extended attempt 3. above and created a messenger object as:

      var messengerContractID = "@mozilla.org/messenger;1";
      var messenger = Components.classes[messengerContractID].createInstance();
      messenger = messenger.QueryInterface(Components.interfaces.nsIMessenger);

Problem now is the following upon rename attempt:
  Error: folderOutliner has no properties (msgMail3PaneWindow.js line 717)

Help. :)

Comment 6

[James Green]

OK I got it to work by copying how the rename dialog does it's job. Problem is
that although it works, it also sends an error message to the console; not too
sure how it can work and error at the same time, I'll attach the patch asap for
someone *hint* to look at -- cc'ing sspitzer.

The error is in RenameFolder() in widgetglue.js - a dump: "no folder tree."

Comment 7

[James Green]

Attachment: Patch for testing

Comment 8

[James Green]

I've fixed the problem, I had left in a broken call. This patch seems to work,
but then I don't have much in the way of servers and folders. Please test.

Should we make the Rename menu items call the properties dialog instead of the
(now) redundant rename dialog?

Comment 9

[[e-mail] (gone - use [e-mail] instead)]

Okay, so I've done some cursory testing of this patch, and I've got to say, it
works fine for me, although I can't truly review the code aspects...  I tried:

Renaming IMAP
Renaming POP
Some sub-folder goodness, too.

Looks good so far.

Comment 10

[James Green]

hwaara, can you review please?

Comment 11

[[e-mail] (gone - use [e-mail] instead)]

Sorry, I also tested the special cases like INBOX (imap), Inbox, Sent (POP3) and
newsgroups, which none of these are renamable.

Comment 12

[Håkan Waara]

Comment on attachment 56001 [details] [diff] [review]
Patch for testing

Looks OK to me. r=hwaara

I think this fits the criterion of being a bug that was being worked on before the semi-lockdown of mailnews.

Comment 13

[(not reading, please use [e-mail] instead)]

I'll get to this some time during the performance lock down.

jglick, do you approve of this UI change?  If so, can you update the spec.

question for james:  what happens if the rename fails?  (say the folder already 
exists)

Comment 14

[James Green]

Seth: if a folder by that name already exists (and we have changed the folder's
name in the dialog), it pops up with an alert saying so, and returns to the
properties dialog. I didn't code the error handling, it was already hooked up in
code that RenameFolder() calls. Just lucky I guess :).

Comment 15

[jglick]

Change sounds fine to me as long as special folders cannot be renamed and the 
correct behavior when a folder name already exists occurs (sounds like we just 
get the same dialog as "Rename Folder".

(Special Folders include:
Inbox
Trash Folder - If the user is using POP or the IMAP "move to Trash" model.  
Sent, Drafts, Templates - based on the user's Copies & Folders Preference 
settings. If the user has their Preferences set so that Sent, Draft and 
Templates messages are saved to a default folder, the folder that is the default 
is special.)

Comment 16

[James Green]

Missed the 0.9.6 boat. Can Seth sr this and check it in when the 0.9.7 tree 
opens?

Comment 17

[Håkan Waara]

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

Comment 18

[Håkan Waara]

This still needs Seth's super-review.  I'll email him about it.

Comment 19

[James Green]

Bouncing out, hope someone with more time than I can catch.

Comment 20

[basic]

thunderbird version of this bug is # 218061

Comment 21

[Stefan Borggraefe]

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

Comment 22

[Scott MacGregor]

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

Comment 23

[Stefan Borggraefe]

Attachment: Updated patch

Comment 24

[Stefan Borggraefe]

Attachment: Better patch

This patch also disables the OK Button when the Name textbox is empty and
catches the exception that is thrown when an illegal folder name was entered.

Comment 25

[Stefan Borggraefe]

Comment on attachment 153133 [details] [diff] [review]
Better patch

Scott, can you review this patch please? Thanks! :-)

Do you think  the Rename Folder dialog should be replaced by this?

Comment 26

[Scott MacGregor]

Comment on attachment 153133 [details] [diff] [review]
Better patch

Making rename bring up the properties dialog would be interesting. Especially
if we could detect this specific case of opening the properties dialog and put
the focus inside the folder checkbox. Lemme think on it for a day. If you don't
hear an answer then assume I decided it was ok to do that. 

On a side note, while trying this patch, I noticed that the folder properties
dialog for my IMAP Inbox folder is extremely wide. For me, it is as wide as the
checkbox text in the General Information tab. The folder name text box is
stretched out to be the width of the dialog as well. I wonder if something
isn't wrapping right in this dialog. 

However when I view other folders it has a normal looking width. Even folders
with longer or shorter names. Do you see anything like this? There's something
abut my imap inbox folder that makes it really wide.

Note: this had nothing to do with your change.

Comment 27

[Stefan Borggraefe]

(In reply to comment #26)
> (From update of attachment 153133 [details] [diff] [review])
> Making rename bring up the properties dialog would be interesting. Especially
> if we could detect this specific case of opening the properties dialog and put
> the focus inside the folder checkbox. Lemme think on it for a day. If you don't
> hear an answer then assume I decided it was ok to do that. 

Ok, I'll file a follow-up bug then (assigned to me).

> On a side note, while trying this patch, I noticed that the folder properties
> dialog for my IMAP Inbox folder is extremely wide. For me, it is as wide as the
> checkbox text in the General Information tab. The folder name text box is
> stretched out to be the width of the dialog as well. I wonder if something
> isn't wrapping right in this dialog. 
> 
> However when I view other folders it has a normal looking width. Even folders
> with longer or shorter names. Do you see anything like this? There's something
> abut my imap inbox folder that makes it really wide.

Looks like the texts on the "Sharing" tab don't wrap. I don't have an IMAP
account which supports sharing, but when I enter a long text manually as a value
for the <label> folderType.text I see the effect you describe. Can you confirm
this is caused by the "Sharing" tab in your case?

Comment 28

[Stefan Borggraefe]

Comment on attachment 153133 [details] [diff] [review]
Better patch

I'll file follow-up bugs for the other issues discussed in this bug. 

This patch should be independent from them and just allows renaming of folders
via the Properties dialog.

Comment 29

[David :Bienvenu]

Comment on attachment 153133 [details] [diff] [review]
Better patch

+    // Initialize name textbox with the given name and remeber this

should be "remember" sr=bienvenu with this spelling fix.

Comment 30

[Scott MacGregor]

The more I think on it, I think we should leave the rename dialog alone.....

Comment 31

[Stefan Borggraefe]

Fix checked in. Thanks for the reviews!

Comment 32

[Scott MacGregor]

(In reply to comment #27)
> Looks like the texts on the "Sharing" tab don't wrap. I don't have an IMAP
> account which supports sharing, but when I enter a long text manually as a value
> for the <label> folderType.text I see the effect you describe. Can you confirm
> this is caused by the "Sharing" tab in your case?

Yup the sharing tab was the trouble maker.