331387 - The AttachFile() should be two function so extensions easier could add an attachment

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Emil Hesslow]

The AttachFile-function always prompt the user for picking a file. If you make an extension that automaticly add attachments it would be better if the function was divided into two function. One for picking file and one for making the file to an attachment that you use for when calling AddAttachment.

Patch is coming.

Comment 1

[Emil Hesslow]

Attachment: Patch #1

I couldn't come up with any better name of the function than makeAttachmentOfFile so I be happy to change it.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Emil, could you please come up with an updated patch for the latest Shredder Trunk builds?

Comment 3

[Wayne Mery (:wsmwk)]

irrc Emil no longer users TB.
assigning => nobody, but would be pleased if Emil comes back :)

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: unbitrotted patch v1

Since Emil is no longer working on this bug, here's the unbitrotted version, if it's still relevant in current environments.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 347724 [details] [diff] [review]
unbitrotted patch v1

The old |var attachments;| should be removed also.

Maybe name it |function AttachFiles(files)| ?

Also please remove the braces for the if clause 

if (fp.show() == nsIFilePicker.returnOK) {

(We prefer not to have any for one line ifs.)

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: second pass

second take with mkmelin's comments incorporated.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 347796 [details] [diff] [review]
second pass

Thx, looks good!

Comment 8

[Magnus Melin [:mkmelin]]

changeset:   1081:666827c06d9d
http://hg.mozilla.org/comm-central/rev/666827c06d9d

->FIXED

Comment 9

[[e-mail]]

IMHO this particular division is not very useful for two reasons:

1. The parameter to AttachFiles is an nsISimpleEnumerator which isn't yet a
   common pattern for JS callers (although jminta's module helps).
2. AttachFiles sets the last attached directory, which is probably wrong for
   use by extensions.

I suggest creating AddFileAttachment and/or AddUrlAttachment methods.

Comment 10

[Magnus Melin [:mkmelin]]

Filed bug 468335 to fix that.