Closed Bug 481667 Opened 11 years ago Closed 11 years ago

Support BCC in message summary files

Categories

(MailNews Core :: Database, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b3

People

(Reporter: rkent, Assigned: rkent)

References

Details

Attachments

(1 file, 2 obsolete files)

To implement the "All Addresses" search functionality in bug 310159, we need to maintain a BCC field for sent email in the message summary file.
Attached patch The fix (obsolete) — Splinter Review
Two issues to be aware of and confirm:

1) I chose not to support aggregate headers in bcc, because I believe that bcc is created by us, and we don't split bcc over multiple headers. Is that OK?

2) I am surprised that I did not have to do anything beyond the ParseMailbox changes to support IMAP. That also works for online IMAP?
Attachment #366629 - Flags: superreview?(bienvenu)
Attachment #366629 - Flags: review?(neil)
Status: NEW → ASSIGNED
Whiteboard: [needs r neil, sr bienvenu]
Target Milestone: --- → Thunderbird 3.0b3
Attachment #366629 - Flags: review?(neil) → review+
Comment on attachment 366629 [details] [diff] [review]
The fix

>-[scriptable, uuid(211b9eef-5f48-4135-aebb-0ac4e7738159)]
>+[scriptable, uuid(EA75203B-2218-4e96-9979-567DFFBEACC2)]
Heh, I don't know whether we have house style for uuid capitalisation ;-)

>+  rv = SetBccList(allRecipients.get());
>+  return rv;
Nit: return SetBccList(allRecipients.get());

>+  bccList    = (m_bccList.length    ? &m_bccList : 0);
>   subject    = (m_subject.length    ? &m_subject    : 0);
>   id         = (m_message_id.length ? &m_message_id : 0);
>   references = (m_references.length ? &m_references : 0);
Nit: line these up?

>diff --git a/mailnews/test/data/bugmail1 b/mailnews/test/data/bugmail1
>--- a/mailnews/test/data/bugmail1
>+++ b/mailnews/test/data/bugmail1
>@@ -30,6 +30,7 @@ To: invalid@example.com
> To: invalid@example.com
> From: PrimaryEmail1@test.invalid
> Cc: invalid@example.com
>+BCC: Some User <u1@example.com>, Another Person <u2@example.com>
> Subject: [Bug 397009] A filter will let me tag, but not untag
> X-Bugzilla-Reason: None
> X-Bugzilla-Type: newchanged
Hmm, wouldn't a draft be a better candidate for a message with a BCC ;-)
Attached patch Fixed Neil's nits (obsolete) — Splinter Review
Fixed Neil's nits, except:

"I don't know whether we have house style for uuid capitalisation"

While lower case is more common, usage of caps is significant. The mostly caps format is what windows guidgen gives, which is a recommended program in MDC for generating uuid. Changing these would be a burden. So I left this alone, unless there is stronger guidance to always use lower case.
Attachment #366629 - Attachment is obsolete: true
Attachment #366841 - Flags: superreview?(bienvenu)
Attachment #366841 - Flags: review+
Attachment #366629 - Flags: superreview?(bienvenu)
Whiteboard: [needs r neil, sr bienvenu] → [needs sr bienvenu]
Comment on attachment 366841 [details] [diff] [review]
Fixed Neil's nits

thx, Kent.

+        if (ret == NS_OK)
+        {

should be if (NS_SUCCEEDED(ret))
Attachment #366841 - Flags: superreview?(bienvenu) → superreview+
Carrying over reviews, patch to checkin.
Attachment #366841 - Attachment is obsolete: true
Attachment #367608 - Flags: superreview+
Attachment #367608 - Flags: review+
Keywords: checkin-needed
Whiteboard: [needs sr bienvenu] → [needs checkin]
Blocks: 483629
Checked in: http://hg.mozilla.org/comm-central/rev/74ee211f58c0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs checkin]
You need to log in before you can comment on or make changes to this bug.