56058 - hardcoded save as draft dialog

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Eric Staats]

From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (WinNT; U)
BuildID:    M17, Netscape PR3

the name of the Drafts folder in the save as draft dialog appears to be 
hardcoded. the message "Message has not been Sent. Do you want to save the 
message in the Drafts folder" assumes the Drafts folder is in fact called Drafts 
when it might not be.

Reproducible: Always
Steps to Reproduce:
1. Create a folder named "My Drafts"
2. In Edit | Mail/News Account Settings | Copies and Folders, select "My Drafts" 
as the folder to which Drafts are saved. 
3. Compose a message, but don't send it.
4. Attempt to close the compose window.
5. Notice the Save as Draft dialog refers to Drafts folder, not the "My Drafts" 
folder.

Expected Results:  Mozilla should have displayed the name of the folder it was 
saving the Draft to.

Comment 1

[Asa Dotzler [:asa]]

Confirmed with 111004 mozilla trunk build.  The File|Save As|Draft also is not
updated, nor are draft messages actually saved to the new folder My Drafts.

Comment 2

[Henrik Lynggaard Hansen]

This bug has not been touched since 2000-11-10, therefore this little reminder.

reporter could you please retest with a currrent build and let us know if this
is still a problem, or the bug can be closed ?

Comment 3

[Eric Staats]

Part two of this bug appears to be fixed. The message is correctly saved to the
"My Drafts" folder.

However, the dialog still says simply Drafts. I think that the way it stands now
is  pretty generic and isn't actually wrong. But it would still be nice to note
the actual folder name, so the user knows the real folder name the message was
saved to.

Comment 4

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 5

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 6

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 7

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 8

[Robert Kaiser]

This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 9

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago.

Because of this, we're resolving the bug as EXPIRED.

If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component.

Query tag for this change: EXPIRED-20100420

Comment 10

[Karsten Düsterloh]

Still valid, although it may be discussable if the term "Drafts" is really the folder name and not just its function.

Comment 11

[:aceman]

(In reply to Karsten Düsterloh from comment #10)
> Still valid, although it may be discussable if the term "Drafts" is really
> the folder name and not just its function.

Exactly. Maybe it should be lowercased so it does not sound like a name.

Comment 12

[Josiah Bruner [:jsbruner]]

So I think we should use the name of the user's chosen folder. It's very possible they call it something other than Drafts entirely. E.G. "For Later", "WIP", "incomplete messages", etc. In which case referring it even to the "drafts" folder would be confusing.

That said, I also am not against switching to "drafts", as they would have to use the drafts settings to switch the folder anyway. 

Basically, if using the actual folder name is not too difficult, I suggest going that route, but if it is, using a lowercased "d" is fine.

Comment 13

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v1

Proposed fix.
Thanks to aceman and JosiahOne for their valuable input.

Comment 14

[Suyash Agarwal (:sshagarwal)]

This bug exists both in TB and SM so I am requesting reviews and ui-reviews accordingly.

Comment 15

[:aceman]

Comment on attachment 8350916 [details] [diff] [review]
Patch v1

Review of attachment 8350916 [details] [diff] [review]:
-----------------------------------------------------------------

Is the change only needed for this single string? Can you look around if there are more occurences of "Drafts folder" in any strings? I think the comments said something about general problem.

All the below TB nits apply to the suite version too.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3326,5 @@
> +    let draftFolderURI = gCurrentIdentity.draftFolder;
> +    let rdf = Components.classes['@mozilla.org/rdf/rdf-service;1']
> +                        .getService(Components.interfaces.nsIRDFService);
> +    let draftFolder = rdf.GetResource(draftFolderURI)
> +                         .QueryInterface(Components.interfaces.nsIMsgFolder);

Do we need rdf here? Can you try MailUtils.getFolderForURI (draftFolderURI) .

@@ +3327,5 @@
> +    let rdf = Components.classes['@mozilla.org/rdf/rdf-service;1']
> +                        .getService(Components.interfaces.nsIRDFService);
> +    let draftFolder = rdf.GetResource(draftFolderURI)
> +                         .QueryInterface(Components.interfaces.nsIMsgFolder);
> +    let draftFolderName = draftFolder.name;

Try .prettyName.

::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
@@ +236,5 @@
>  
>  ## Strings use for the save message dialog shown when the user close a message compose window
>  saveDlogTitle=Save Message
> +## LOCALIZATION NOTE (SaveDlogMessages): %1$S is folder name
> +saveDlogMessages=Message has not been sent. Do you want to save the message in your Drafts folder (%1$S)?

I think we wanted "drafts folder" (lowercased).

Comment 16

[Suyash Agarwal (:sshagarwal)]

Attachment: Patch v2

Sorry for "...Drafts...".
I couldn't find any other "Draft" usage in composeMsgs.properties that would need this generic draft folder name.

Thanks.

Comment 17

[:aceman]

Comment on attachment 8350919 [details] [diff] [review]
Patch v2

Review of attachment 8350919 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't run tested this but it looks nice now.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3328,4 @@
>      let result = Services.prompt
>                           .confirmEx(window,
>                                      getComposeBundle().getString("saveDlogTitle"),
> +                                    getComposeBundle().getFormattedString("saveDlogMessages",[draftFolderName]),

Space after comma.

::: suite/mailnews/compose/MsgComposeCommands.js
@@ +2021,3 @@
>      switch (Services.prompt.confirmEx(window,
>                sComposeMsgsBundle.getString("saveDlogTitle"),
> +              sComposeMsgsBundle.getFormattedString("saveDlogMessages",[draftFolderName]),

Space after comma.

Comment 18

[Josiah Bruner [:jsbruner]]

Comment on attachment 8350919 [details] [diff] [review]
Patch v2

Review of attachment 8350919 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks and Merry Christmas!

Comment 19

[[e-mail]]

Comment on attachment 8350919 [details] [diff] [review]
Patch v2

I haven't tested this but it seems simple enough. (Famous last words...)

The L10N note has the wrong capitalisation of the string name.

Don't forget those space after comma that aceman mentioned.

I don't like your tweak to the string name, instead I would change it to saveDlgText instead. r=me with those fixed.

You could just use %S since there is only one string, although I see that %1$S is popular too.

Comment 20

[[e-mail]]

I've checked the rest of the SeaMonkey UI (which should be pretty representative of the Thunderbird UI in this case) and we seem to use the special folder names in the following places:
1. Help
2. Account Settings
3. Confirming a deferred account
4. Closing the compose window
5. Empty Junk/Trash
6. Delete no Trash

Except obviously in Account Settings, in none of these cases do I see any attempt to use the actual folder name, so we are currently consistent, at least...

(Also, some localisers keep the capitalisation on the special folder name and others don't.)

Should we, for instance, change the label on the "Empty Trash" menuitem in case the Trash folder is actually called "Deleted Items"? Should we change it to "Empty trash (Deleted Items)" or "Empty Deleted Items"? (Fortunately, at least in en-US, the access key is a letter from the word Empty.) Oh, and I'll just throw a third alternative into the mix: "Empty Trash" if the trash folder name is (localised) Trash, but "Empty trash (Deleted Items)" if it isn't.

Comment 21

[Magnus Melin [:mkmelin]]

Comment on attachment 8350919 [details] [diff] [review]
Patch v2

Review of attachment 8350919 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok to me. Sorry for the delay.

Comment 22

[Philip Chee]

Comment on attachment 8350919 [details] [diff] [review]
Patch v2

looks good.
> +    let draftFolderName = MailUtils.getFolderForURI(draftFolderURI).prettyName;

When do we use .prettiestName and when do we use .prettyName ?

r=me if you explain the above naming nit.

Comment 23

[:aceman]

The explanation could be that /suite does never use .prettiestName :) http://mxr.mozilla.org/comm-central/search?string=prettiestName&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

Comment 24

[Philip Chee]

(In reply to [e-mail] from comment #20)
> I've checked the rest of the SeaMonkey UI (which should be pretty
> representative of the Thunderbird UI in this case) and we seem to use the
> special folder names in the following places:
> 1. Help
> 2. Account Settings
> 3. Confirming a deferred account
> 4. Closing the compose window
> 5. Empty Junk/Trash
> 6. Delete no Trash
> 
> Except obviously in Account Settings, in none of these cases do I see any
> attempt to use the actual folder name, so we are currently consistent, at
> least...
> 
> (Also, some localisers keep the capitalisation on the special folder name
> and others don't.)
> 
> Should we, for instance, change the label on the "Empty Trash" menuitem in
> case the Trash folder is actually called "Deleted Items"? Should we change
> it to "Empty trash (Deleted Items)" or "Empty Deleted Items"? (Fortunately,
> at least in en-US, the access key is a letter from the word Empty.) Oh, and
> I'll just throw a third alternative into the mix: "Empty Trash" if the trash
> folder name is (localised) Trash, but "Empty trash (Deleted Items)" if it
> isn't.

Out of scope aren't we? I think we should discuss this in the dev newsgroup.

Comment 25

[Suyash Agarwal (:sshagarwal)]

So, this is a good point that we should possibly change the menuitem as well when the folder is renamed. But that can be a followup bug. So, marking this for check-in.
If someone has any issues with this, please let me know.

Thanks.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/07ac824985d0