Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Status

SeaMonkey
MailNews: Composition
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
I'm going to port the following patches in one go because they all touch composeStartup() and it's easier to do it in one go:

[Bug 384160] Cannot send attachments with cli and file with accents.
  [Bug 524428] "thunderbird -compose attachment=file://..." fails to send if the attachment file url is encoded.
[Bug 530779] With main window/3-pane closed, sent email cannot be copied to "sent"-folder and drafts cannot be saved - "There was an error copying the message to the Sent folder. Retry?"
[Bug 295956] Editing message in drafts folder (with IMAP and multiple profiles) results in error.
(Assignee)

Comment 1

7 years ago
Created attachment 495511 [details] [diff] [review]
Patch v1.0 Port stuff.

> [Bug 530779] With main window/3-pane closed, sent email cannot be copied to
> "sent"-folder and drafts cannot be saved - "There was an error copying the
> message to the Sent folder. Retry?"
I didn't port the Addressbook part of this patch here. Should I do it here or in some other bug/patch?

> [Bug 295956] Editing message in drafts folder (with IMAP and multiple profiles)
> results in error.
I only ported the first half of this patch. I couldn't identify the equivalent code for the rest of the patch.
Attachment #495511 - Flags: review?(iann_bugzilla)

Comment 2

7 years ago
(In reply to comment #1)
> Created attachment 495511 [details] [diff] [review]
> Patch v1.0 Port stuff.
> 
> > [Bug 530779] With main window/3-pane closed, sent email cannot be copied to
> > "sent"-folder and drafts cannot be saved - "There was an error copying the
> > message to the Sent folder. Retry?"
> I didn't port the Addressbook part of this patch here. Should I do it here or
> in some other bug/patch?
It needs doing whilst we think about it, whichever is easier/more manageable for you.
> 
> > [Bug 295956] Editing message in drafts folder (with IMAP and multiple profiles)
> > results in error.
> I only ported the first half of this patch. I couldn't identify the equivalent
> code for the rest of the patch.
That code was added to TB in Bug 214687, so depends on whether we want that feature - Mnyromyr?

Comment 3

7 years ago
Comment on attachment 495511 [details] [diff] [review]
Patch v1.0 Port stuff.

r=me
Second part of that patch is not needed for SM as you pointed out, so just the AB part to be done of the other bug.
Attachment #495511 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 4

7 years ago
Created attachment 497782 [details] [diff] [review]
Patch v1.1 Include Address Book bits. r=IanN

>> I didn't port the Addressbook part of this patch here. Should I do it here or
>> in some other bug/patch?
> It needs doing whilst we think about it, whichever is easier/more manageable
> for you.

> Ian Neal      2010-12-14 06:47:17 PST
> 
> r=me
> Second part of that patch is not needed for SM as you pointed out, so just the
> AB part to be done of the other bug.

OK. New patch will include Address book bits.
Attachment #495511 - Attachment is obsolete: true
Attachment #497782 - Flags: superreview?(neil)
Attachment #497782 - Flags: review+

Comment 5

7 years ago
Comment on attachment 497782 [details] [diff] [review]
Patch v1.1 Include Address Book bits. r=IanN

>+var msgWindow = Components.classes["@mozilla.org/messenger/msgwindow;1"]
>+                          .createInstance(Components.interfaces.nsIMsgWindow);
So what does this actually achieve?

>+          if (/^file:\/\//i.test(attachmentStr))
>+          {
>+            attachment.url = encodeURI(attachmentStr);
>+          }
>+          else
>+          {
>+            localFile.initWithPath(attachmentStr);
>+            attachment.url = fileHandler.getURLSpecFromFile(localFile);;
[Nit: ;;]
I would actually do this the other way around: assume that attachmentStr is a path, and if the init fails, fall back to treating it as a URL.
(Assignee)

Updated

7 years ago
Blocks: 455870
(Assignee)

Comment 6

7 years ago
Created attachment 498012 [details] [diff] [review]
Patch v1.2 Fix review issues. r=IanN

>>+          if (/^file:\/\//i.test(attachmentStr))
>>+          {
>>+            attachment.url = encodeURI(attachmentStr);
>>+          }
>>+          else
>>+          {
>>+            localFile.initWithPath(attachmentStr);
>>+            attachment.url = fileHandler.getURLSpecFromFile(localFile);;
> [Nit: ;;]
> I would actually do this the other way around: assume that attachmentStr is a
> path, and if the init fails, fall back to treating it as a URL.
Fixed.

Tested with:
-compose "attachment='file:///C:/T1/aéa.txt'"
-compose attachment=file:///C:/T1/t%C3%A9%C3%A0%C3%A8%C3%B9.txt
-compose "attachment='c:\T1\aéa.txt'"

>>+var msgWindow = Components.classes["@mozilla.org/messenger/msgwindow;1"]
>>+                          .createInstance(Components.interfaces.nsIMsgWindow);
> So what does this actually achieve?

Looking more closely at Bug 530779 Comment 36:
> oh, I should say, this bug won't occur on the mac, or w/ SeaMonkey.

So this is not needed (retested without this part of the patch), but then why do we have this in our mail3pane window?
<http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindow.js?mark=167-167,194-194#167>
Attachment #497782 - Attachment is obsolete: true
Attachment #498012 - Flags: superreview?(neil)
Attachment #498012 - Flags: review+
Attachment #497782 - Flags: superreview?(neil)

Comment 7

7 years ago
(In reply to comment #6)
> why do we have this in our mail3pane window?
> <http://mxr.mozilla.org/comm-central/source/suite/mailnews/mailWindow.js?mark=167-167,194-194#167>
Because we need e.g. to add a folder listener to it.

According to bug 530779, Thunderbird also uses it to detect shutdown, but I've no idea why it needs to do it like that.

Comment 8

7 years ago
Comment on attachment 498012 [details] [diff] [review]
Patch v1.2 Fix review issues. r=IanN

>+            attachment.url = fileHandler.getURLSpecFromFile(localFile);;
Nit: ;;
Attachment #498012 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 9

7 years ago
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/c6c5290dc2ae
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

6 years ago
Blocks: 663193
You need to log in before you can comment on or make changes to this bug.