Closed Bug 464651 Opened 16 years ago Closed 15 years ago

[Port Bug 331387] AttachFile() should be split into two so that extensions can more easily add attachments

Categories

(SeaMonkey :: MailNews: Composition, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0b1

People

(Reporter: philip.chee, Assigned: misak.bugzilla)

References

Details

Attachments

(1 file, 4 obsolete files)

+++ This bug was initially created as a clone of Bug #331387 +++

The AttachFile() function always prompts the user to pick a file. If you make an extension that automatically add attachments it would be better if this function was divided into two; one for picking the file and one for making the file an attachment that you can use when calling AddAttachment().
Summary: [Port Bug 331387] The AttachFile() should be two function so extensions easier could add an attachment → [Port Bug 331387] AttachFile() should be split into two so that extensions can more easily add attachments
Attached patch patch (obsolete) — Splinter Review
Straightforward port
Attachment #351134 - Flags: superreview?(neil)
Attachment #351134 - Flags: review?(neil)
Assignee: nobody → misak
Comment on attachment 351134 [details] [diff] [review]
patch

AttachFile also saves the last attached directory and we don't want an extension to accidentally change it. (The original patch might have the same bug.)
Attachment #351134 - Flags: superreview?(neil)
Attachment #351134 - Flags: superreview-
Attachment #351134 - Flags: review?(neil)
Depends on: 468335
Attached patch v 2 (obsolete) — Splinter Review
I've changed AttachFiles to return first file and doing SetLastAttachDirectory in AttachFile.
Attachment #351134 - Attachment is obsolete: true
Attachment #364082 - Flags: superreview?(neil)
Attachment #364082 - Flags: review?(neil)
Status: NEW → ASSIGNED
Comment on attachment 364082 [details] [diff] [review]
v 2

>+  var lastAttachDirectory = null;
But this is neither a directory nor the last attachment...
Comment on attachment 364082 [details] [diff] [review]
v 2

>+  var lastAttachDirectory = null;
How about firstAttachedFile?

>     if (!haveSetAttachDirectory) {
You can use !firstAttachedFile here, and eliminate haveSetAttachDirectory.
Comment on attachment 364082 [details] [diff] [review]
v 2

r+sr=me with those changes.
Attachment #364082 - Flags: superreview?(neil)
Attachment #364082 - Flags: superreview+
Attachment #364082 - Flags: review?(neil)
Attachment #364082 - Flags: review+
Attached patch for checkin (obsolete) — Splinter Review
same as above with Neil comments fixed.
Attachment #364082 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 364097 [details] [diff] [review]
for checkin

(In reply to Comment 6)
> (From update of attachment 364082 [details] [diff] [review])
> r+sr=me with those changes.

Misak, I thought you had editbugs these days. Anyway carrying forward r+/sr+ from Comment 6
Attachment #364097 - Flags: superreview+
Attachment #364097 - Flags: review+
Actually Misak forgot to s/lastAttachDirectory/firstAttachedFile/ everywhere.
Keywords: checkin-needed
Attachment #364097 - Flags: superreview+
Attachment #364097 - Flags: review+
Attached patch proper for checkin (obsolete) — Splinter Review
Sorry :(
Attachment #364097 - Attachment is obsolete: true
Comment on attachment 364274 [details] [diff] [review]
proper for checkin

carrying forward r+ sr+ from Neil.
Attachment #364274 - Flags: superreview+
Attachment #364274 - Flags: review+
Now really s/lastAttachDirectory/firstAttachedFile/ everywhere :( Carrying forward r+ and sr+ from Neil.
Attachment #364274 - Attachment is obsolete: true
Attachment #364306 - Flags: superreview+
Attachment #364306 - Flags: review+
Keywords: checkin-needed
Comment on attachment 364306 [details] [diff] [review]
really final for checkin
[Checkin: Comment 13]


http://hg.mozilla.org/comm-central/rev/a788df7a01db
Attachment #364306 - Attachment description: really final for checkin → really final for checkin [Checkin: Comment 13]
No longer blocks: TB2SM
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: