Closed
Bug 464651
Opened 16 years ago
Closed 16 years ago
[Port Bug 331387] AttachFile() should be split into two so that extensions can more easily add attachments
Categories
(SeaMonkey :: MailNews: Composition, enhancement)
SeaMonkey
MailNews: Composition
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.0b1
People
(Reporter: philip.chee, Assigned: misak.bugzilla)
References
Details
Attachments
(1 file, 4 obsolete files)
|
(deleted),
patch
|
misak.bugzilla
:
review+
misak.bugzilla
:
superreview+
|
Details | Diff | Splinter Review |
+++ 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().
|
Reporter | |
Updated•16 years ago
|
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
|
Assignee | |
Comment 1•16 years ago
|
||
Straightforward port
Attachment #351134 -
Flags: superreview?(neil)
Attachment #351134 -
Flags: review?(neil)
|
|
Reporter | |
Updated•16 years ago
|
Assignee: nobody → misak
|
||
Comment 2•16 years ago
|
||
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)
|
|
Assignee | |
Comment 3•16 years ago
|
||
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)
|
|
Assignee | |
Updated•16 years ago
|
Status: NEW → ASSIGNED
|
|
||
Comment 4•16 years ago
|
||
Comment on attachment 364082 [details] [diff] [review]
v 2
>+ var lastAttachDirectory = null;
But this is neither a directory nor the last attachment...
|
|
||
Comment 5•16 years ago
|
||
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 6•16 years ago
|
||
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+
|
|
Assignee | |
Comment 7•16 years ago
|
||
same as above with Neil comments fixed.
Attachment #364082 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•16 years ago
|
Keywords: checkin-needed
|
|
Reporter | |
Comment 8•16 years ago
|
||
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+
|
|
||
Comment 9•16 years ago
|
||
Actually Misak forgot to s/lastAttachDirectory/firstAttachedFile/ everywhere.
Keywords: checkin-needed
|
|
Reporter | |
Updated•16 years ago
|
Attachment #364097 -
Flags: superreview+
Attachment #364097 -
Flags: review+
|
|
Assignee | |
Comment 11•16 years ago
|
||
Comment on attachment 364274 [details] [diff] [review]
proper for checkin
carrying forward r+ sr+ from Neil.
Attachment #364274 -
Flags: superreview+
Attachment #364274 -
Flags: review+
|
|
Assignee | |
Comment 12•16 years ago
|
||
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+
|
|
Assignee | |
Updated•16 years ago
|
Keywords: checkin-needed
|
||
Comment 13•16 years ago
|
||
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]
|
|
||
Updated•16 years ago
|
No longer blocks: TB2SM
Status: ASSIGNED → RESOLVED
Closed: 16 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.
Description
•