Closed Bug 56058 Opened 19 years ago Closed 6 years ago

hardcoded save as draft dialog

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.27

People

(Reporter: staats, Assigned: sshagarwal)

Details

(Whiteboard: [Halloween2011Bug])

Attachments

(1 file, 1 obsolete file)

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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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 ?
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.
Product: Browser → Seamonkey
Assignee: scottputterman → nobody
Priority: P3 → --
QA Contact: esther → message-display
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
Status: NEW → UNCONFIRMED
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
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
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
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
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
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → EXPIRED
Status: RESOLVED → UNCONFIRMED
Resolution: EXPIRED → ---
Still valid, although it may be discussable if the term "Drafts" is really the folder name and not just its function.
Severity: trivial → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Halloween2011Bug]
(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.
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.
Attached patch Patch v1 (obsolete) — Splinter Review
Proposed fix.
Thanks to aceman and JosiahOne for their valuable input.
Assignee: nobody → syshagarwal
Status: NEW → ASSIGNED
Attachment #8350916 - Flags: ui-review?(neil)
Attachment #8350916 - Flags: ui-review?(josiah)
Attachment #8350916 - Flags: review?(neil)
Attachment #8350916 - Flags: review?(mkmelin+mozilla)
Attachment #8350916 - Flags: feedback?(acelists)
This bug exists both in TB and SM so I am requesting reviews and ui-reviews accordingly.
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).
Attachment #8350916 - Flags: feedback?(acelists) → feedback-
Attached patch Patch v2Splinter Review
Sorry for "...Drafts...".
I couldn't find any other "Draft" usage in composeMsgs.properties that would need this generic draft folder name.

Thanks.
Attachment #8350916 - Attachment is obsolete: true
Attachment #8350916 - Flags: ui-review?(neil)
Attachment #8350916 - Flags: ui-review?(josiah)
Attachment #8350916 - Flags: review?(neil)
Attachment #8350916 - Flags: review?(mkmelin+mozilla)
Attachment #8350919 - Flags: ui-review?(neil)
Attachment #8350919 - Flags: ui-review?(josiah)
Attachment #8350919 - Flags: review?(neil)
Attachment #8350919 - Flags: review?(mkmelin+mozilla)
Attachment #8350919 - Flags: feedback?(acelists)
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.
Attachment #8350919 - Flags: feedback?(acelists) → feedback+
Comment on attachment 8350919 [details] [diff] [review]
Patch v2

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

Looks good. Thanks and Merry Christmas!
Attachment #8350919 - Flags: ui-review?(josiah) → ui-review+
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.
Attachment #8350919 - Flags: review?(neil) → review+
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 on attachment 8350919 [details] [diff] [review]
Patch v2

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

Looks ok to me. Sorry for the delay.
Attachment #8350919 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8350919 - Flags: review?(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.
Attachment #8350919 - Flags: review?(philip.chee) → review+
(In reply to neil@parkwaycc.co.uk 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.
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.
Keywords: checkin-needed
Attachment #8350919 - Flags: ui-review?(neil)
https://hg.mozilla.org/comm-central/rev/07ac824985d0
Status: ASSIGNED → RESOLVED
Closed: 9 years ago6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.27
You need to log in before you can comment on or make changes to this bug.