Closed Bug 654966 Opened 13 years ago Closed 13 years ago

File > Save As > Template should not be active for NNTP folders.

Categories

(SeaMonkey :: MailNews: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: ewong)

References

Details

Attachments

(1 file, 3 obsolete files)

From 654007 Comment 13:

> Karsten Düsterloh      2011-05-04 14:11:03 PDT
> 
>>> It does work if the message to be saved is local/Movemail/RSS/POP3 or IMAP,
>>> but it's still broken if I try to template an NNTP message.
>> 
>> That's because |nsIMsgFolder::createStorageIfMissing()| isn't implemented for
>> NNTP accounts :P.
> 
> That's not the full story. For me, 
> | MailUtils.getFolderForURI(identity.stationeryFolder, false);
> delivers "Local Folders/Templates".
> 
> Maybe NNTP accounts are just "too special", because I doubt that Template
> management for NNTP via Local Folders would be particularily useful. :-/
> You got a point here, true.
> 
>> Do you still want me to file a NNTP bug?
> 
> Yes, because the menu item shouldn't be active if it doesn't do anything
> useful.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #559920 - Flags: review?(iann_bugzilla)
Comment on attachment 559920 [details] [diff] [review]
Hide Template menu item for news items.

If you were going to go down the disabling route then you should probably do it in mail3PaneWindowCommands.js and messageWindow.js via isCommandEnabled for cmd_saveAsTemplate
I agree the current situation of failing silently is wrong. I believe the better long term solution is being able to save as a template to the location configured in Mail & Newsgroups Account Settings. Simply disabling the "save as template" is short term.
Attachment #559920 - Flags: review?(iann_bugzilla) → review-
Attachment #559920 - Attachment is obsolete: true
Attachment #563285 - Flags: review?(iann_bugzilla)
Comment on attachment 563285 [details] [diff] [review]
Disabled File > Save As > Template for NNTP folders.

>+++ b/suite/mailnews/messageWindow.js
>@@ -739,16 +739,18 @@ var MessageWindowController =
> 			case "cmd_forwardInline":
> 			case "cmd_forwardAttachment":
> 			case "cmd_editAsNew":
> 			case "cmd_print":
> 			case "cmd_printpreview":
>       case "button_print":
> 			case "cmd_saveAsFile":
> 			case "cmd_saveAsTemplate":
>+        var target = gMessageBrowser.contentPrincipal.URI.scheme;
>+        return !(target == "news");
You've missed the fact all the cases above this one drop through, thus you'll make these disabled under these conditions too.
You will need to return the correct value after case "cmd_saveAsFile"

> 			case "cmd_viewPageSource":
> 			case "cmd_reload":
> 			case "cmd_find":
>       case "button_mark":
> 			case "cmd_markAsRead":
> 			case "cmd_markAllRead":
> 			case "cmd_markThreadAsRead":
> 			case "cmd_markReadByDate":

r- for the moment.
Attachment #563285 - Flags: review?(iann_bugzilla) → review-
Attachment #563285 - Attachment is obsolete: true
Attachment #564033 - Flags: review?(iann_bugzilla)
Comment on attachment 564033 [details] [diff] [review]
Disable File > Save As > Template for NNTP folders (v3).

>       case "cmd_saveAsTemplate":
>+        var target = gMessageBrowser.contentPrincipal.URI.scheme;
>+        if (( GetNumSelectedMessages() > 1) || (target == "news"))
Nit: Could you remove the space between ( and Get
Nit: You probably have more pairs of brackets/braces than is strictly necessary.

> 			case "cmd_saveAsTemplate":
>+        var target = gMessageBrowser.contentPrincipal.URI.scheme;
>+        return !(target == "news");
Nit: I'd prefer
return target != "news";

r=me with those issues addressed.
Attachment #564033 - Flags: review?(iann_bugzilla) → review+
Fixed nits from comment 6.
Attachment #564033 - Attachment is obsolete: true
Attachment #564164 - Flags: review+
Keywords: checkin-needed
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/c6a8260db0aa
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Keywords: checkin-needed
Depends on: 765789
You need to log in before you can comment on or make changes to this bug.