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)
SeaMonkey
MailNews: General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: philip.chee, Assigned: ewong)
References
Details
Attachments
(1 file, 3 obsolete files)
1.97 KB,
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•13 years ago
|
||
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-
Assignee | ||
Comment 3•13 years ago
|
||
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-
Assignee | ||
Comment 5•13 years ago
|
||
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+
Assignee | ||
Comment 7•13 years ago
|
||
Fixed nits from comment 6.
Attachment #564033 -
Attachment is obsolete: true
Attachment #564164 -
Flags: review+
Reporter | ||
Updated•13 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 8•13 years ago
|
||
Pushed to comm-central: http://hg.mozilla.org/comm-central/rev/c6a8260db0aa
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•13 years ago
|
Keywords: checkin-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•