Convert nsMsgCompose::BuildMailListArray nsISupportsArray argument to something better

RESOLVED FIXED in Thunderbird 21.0

Status

MailNews Core
Composition
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: aceman, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
Thunderbird 21.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

8.38 KB, patch
standard8
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
nsresult nsMsgCompose::BuildMailListArray(nsIAbDirectory* parentDir,
                                          nsISupportsArray* array)

and

nsresult nsMsgCompose::GetMailListAddresses(nsString& name, nsISupportsArray* mailListArray, nsIMutableArray** addressesArray)
(Reporter)

Comment 1

5 years ago
Created attachment 705567 [details] [diff] [review]
patch
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #705567 - Flags: review?(neil)
Attachment #705567 - Flags: feedback?(mkmelin+mozilla)
(Reporter)

Updated

5 years ago
Blocks: 834020

Comment 2

5 years ago
Comment on attachment 705567 [details] [diff] [review]
patch

Review of attachment 705567 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok afaikt.
Attachment #705567 - Flags: feedback?(mkmelin+mozilla) → feedback+
(Assignee)

Comment 3

5 years ago
Comment on attachment 705567 [details] [diff] [review]
patch

>       nsMsgMailList* mailList;
>-      rv = enumerator->CurrentItem((nsISupports**)&mailList);
>+      rv = enumerator->GetNext((nsISupports**)&mailList);
I'm sorry but I can't perpetuate this horror, the code needs a rewrite.
Attachment #705567 - Flags: review?(neil) → review-
(Assignee)

Comment 4

5 years ago
Created attachment 705819 [details] [diff] [review]
Proposed patch

* Moved more of the fullname init code into the constructor
* Changed the array from an nsISupportsArray to an nsTArray
* Rewrote GetMailListAddresses to use an IndexOf comparator
Assignee: acelists → neil
Attachment #705567 - Attachment is obsolete: true
Attachment #705819 - Flags: review?(mbanner)
(Reporter)

Comment 5

5 years ago
Quite involved rewrite, even the class changed to struct :)
Summary: Convert nsMsgCompose::BuildMailListArray nsISupportsArray argument to nsIMutableArray → Convert nsMsgCompose::BuildMailListArray nsISupportsArray argument to something better
Comment on attachment 705819 [details] [diff] [review]
Proposed patch

Review of attachment 705819 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, and the unit tests pass :-)
Attachment #705819 - Flags: review?(mbanner) → review+
(Reporter)

Updated

5 years ago
Blocks: 838805
(Assignee)

Comment 7

5 years ago
Pushed comm-central changeset c52e29d0ff20.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 21.0
You need to log in before you can comment on or make changes to this bug.