92726 - the Save As filename should default to the Subject of the message

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[Marcus]

Now this is a trivial thing that helps a lot:
It would make saving a number of mails much easier if
the subject-line became the default-name in save-as
and if it would remember the file-type one selected
the last time as it remembers the path(saving a number
of related mails as .txt requires to select .txt and
enter the file-name every time again).

Comment 1

[Mike Schuette]

This seems like a rfe bug filed on 2 separate topics:

* Make the filename the Subject (would require some significant munging,
methinks, and isn't quite so "trivial", but I might be wrong.)

* Make the file type "sticky"

The summary poorly describes both, so I'm updating it to reflect the first RFE.
 (old summary was "filename and type in save-as")

Note, I haven't searched exhaustively for a dupe on either subject.  Feel free
to file a 2nd bug for the 2nd request, but please check for dupes first.

Comment 2

[Robert Wall]

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

Comment 3

[:aceman]

Voting for both parts of this report, especially the 2nd:)

Comment 4

[:aceman]

Actually, the second part is already done in some way. The actual type of the
saved file is determined from the extension of the filename you specify, not the
type of files you select in the list. If you just press Ctrl+S, type message.txt
and leave the type as .eml files, hit Enter, the mail is saved as plain text file.

I don't know if this is a bug, but I like it as a feature:)

Comment 5

[Mike Cowperthwaite]

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

Comment 6

[Thomas 'PointedEars' Lahn]

Voting for this bug.

Munging the subject for a valid filename should not be that difficult (as KNode
0.7.x does it, too, which is why I missed the feature in MailNews in the first
place.)

Replace problematic characters[1] with underscore (`_'). Optionally replace two
juxtapositioned underscores with one. Strip leading and following underscores,
then add the default extension.

[1]
Problematic characters include `\', `/', `:', `*', `?', `"', `<', `>', and `|'
because (from what Explorer.exe tells me) they must not appear in filenames at
least on a FAT or NTFS partition.


\V/ Live long and prosper

PointedEars

Comment 7

[Peter Wilson]

Additional Rationale for this RFE:
I am adding automated racer registration for ski racing events.  Pre-formatted
emails are directed to the race organizer for a particular event.
After saving the email the local copy of the registration program processes the
contents to register the racer. I ensure that each message subject has a unique
number affixed. 
In Outlook Express the race organizer need only use SaveAs (Ctrl S + ENTER) for
each of about 120 messages. In Mozilla the Save As name prompt is always
"message.eml" - which creates a huge increase in effort. 
I presume the SaveAs directory will be sticky as well.

Comment 8

[Ian Neal]

Taking from [e-mail] (Seth Spitzer)

Comment 9

[Ian Neal]

Accepting

Comment 10

[Ian Neal]

Attachment: Patch v0.1 - implements using subject for mail/compose "save as file" filename

This patch does several things:
Removes most of the duplication of TrimString by moving it to globalOverlay.js
Adds a new function GenerateValidFilename to globalOverlay.js for both mailnews
and editor to use which was originally part of editor's GetSuggestedFilename
Adds an extra argument to editor's SaveDocument to pass the subject via, if the
extra argument is null then reverts to old behaviour
Fixed calls to SaveDocument from elsewhere so extra argument is null
Adds an extra argument to editor's GetSuggestedFilename so editorType is passed

Reworks GetSuggestedFilename so it calls new GenerateValidFilename
Alters fourth argument of nsMessenger's SaveAs to be used for passing the
subject.

Comment 11

[[e-mail]]

Comment on attachment 133152 [details] [diff] [review]
Patch v0.1 - implements using subject for mail/compose "save as file" filename

Firstly, in the message compose case, wouldn't it be somewhat simpler to set
the document title from the subject? Then the editor save routine would
automagically suggest the correct filename.

Secondly, for the message display case, the content won't have the correct
title if the current message was changed while the preview pane was closed.

Comment 12

[Ian Neal]

Attachment: Patch v0.2 - use subject for compose/mail "Save As File" filename

This patch does several things:
Removes most of the duplication of TrimString by moving it to globalOverlay.js
Adds a new function GenerateValidFilename to globalOverlay.js for both mailnews

and editor to use which was originally part of editor's GetSuggestedFilename
Uses .setDocumentTitle to pass the subject to editor's SaveDocument
Reworks GetSuggestedFilename so it calls new GenerateValidFilename
Alters fourth argument of nsMessenger's SaveAs to be used for passing the
subject.

Comment 13

[[e-mail]]

Comment on attachment 133227 [details] [diff] [review]
Patch v0.2 - use subject for compose/mail "Save As File" filename

Looking good.

>+  if (title == "")
>+    title = GetCurrentEditor().document.title;
Move XPCNativeWrapper.js into editorOverlay.xul as discussed on IRC.

> function SaveAsFile(uri)
> {
>-	if (uri) messenger.saveAs(uri, true, null, msgWindow);
>+  if (uri) {
>+    var subject = null;
>+    try {
>+      subject = messenger.messageServiceFromURI(uri)
>+                         .messageURIToMsgHdr(uri).subject;
>+      subject = GenerateValidFilename(subject, ".eml");
>+    }
>+    catch (ex) {}
>+    messenger.saveAs(uri, true, null, subject);
As the 4th arg is really a suggested filename rather than a subject, declare
two variables (you can declare subject inside the try block), and rename the
4th arg in the rest of the patch too.

Comment 14

[Ian Neal]

Attachment: Patch v0.2a - uses subject for compose/mail "Save As File" filename

Addresses Neil's review comments.
I presume that globalOverlay.js is a suitable place to move TrimString to and
add GenerateValidFilename to?

Comment 15

[Ian Neal]

Attachment: Patch v0.2b - implements use of subject for "Save As File" filename

As per patch v0.2a but functions moved from globalOverlay.js to
utilityOverlay.js, editorUtilities.js is not referenced by messenger.xul so
isn't suitable.

Comment 16

[Ian Neal]

Comment on attachment 133232 [details] [diff] [review]
Patch v0.2a - uses subject for compose/mail "Save As File" filename

Cancelling old review request

Comment 17

[[e-mail]]

Comment on attachment 133286 [details] [diff] [review]
Patch v0.2b - implements use of subject for "Save As File" filename

r=me based on visual differences between this patch and attachment 133232 [details] [diff] [review] and
my previous testing of bug 133232. Make sure to get some sort of editor moa too
(I'm guessing you'll get any mailnews moa by sr).

Comment 18

[[e-mail]]

Um, make that attachment 133232 [details] [diff] [review] both times :-[

Comment 19

[Ian Neal]

Attachment: Patch v0.2c - as v0.2b except with trimstring change

TrimString has been left where it originally was in editorUtilities.js and the
call to it from GenerateValidFilename has been inlined.

Comment 20

[David :Bienvenu]

Comment on attachment 133364 [details] [diff] [review]
Patch v0.2c - as v0.2b except with trimstring change

sr=bienvenu, but you should get an editor person to do an r= for the editor
changes - maybe [e-mail]

\ No newline at end of file
+}

could you add a newline here so that message won't come up in the diffs?

Comment 21

[Ian Neal]

Adding Daniel to CC to make sure there's no problems from the editor side of things

Comment 22

[Ian Neal]

David,

\ No newline at end of file
+}
I'm pretty sure this happens because there was no newline at the end of the file
before.

Comment 23

[Ian Neal]

Comment on attachment 133364 [details] [diff] [review]
Patch v0.2c - as v0.2b except with trimstring change

Requesting review for the editor side from Daniel.
Already got
r=neil.parkwaycc.co.uk for the mail side.

Comment 24

[David :Bienvenu]

Ian, yes, you're right, it was that way before, I had checked - but I was hoping
you could fix it anyway :-)

Comment 25

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 133364 [details] [diff] [review]
Patch v0.2c - as v0.2b except with trimstring change

r=[e-mail] for the editor side; seems ok to me

Comment 26

[Ian Neal]

Comment on attachment 133364 [details] [diff] [review]
Patch v0.2c - as v0.2b except with trimstring change

Requesting driver approval for checkin to trunk

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 133364 [details] [diff] [review]
Patch v0.2c - as v0.2b except with trimstring change

a=asa (on behalf of drivers) for checkin to 1.6alpha

Comment 28

[Scott MacGregor]

while reading your GenerateValidFilename(), I was reminded of
NS_MsgHashIfNecessary()

http://lxr.mozilla.org/mozilla/source/mailnews/base/util/nsMsgUtils.cpp#240

some characters are valid on some platforms but not others. 

see http://lxr.mozilla.org/mozilla/source/xpcom/ds/nsCRT.h#276

I'll bet there are more places in C++ (and js) where we do this sort thing.

we should both:

1) make sure the code your adding is doing the right thing on all platforms
2) log a bug about unifying all the core of this code in one place, accessible
from C++ and js.

Comment 29

[[e-mail]]

Fix checked in.

BTW, the appropriate filepickers will perform additional filename cleanup.

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

generatevalidfilename seems to do the same job as the existing validateFileName.
filed Bug 223549 to use that existing function instead of adding a new one

Comment 31

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using SeaMonkey 1.5a;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060110 Mozilla/1.0.

I checked News, IMAP, and POP/Local Folders.