Closed
Bug 467538
Opened 16 years ago
Closed 16 years ago
Replace nsIMsgRecipientArray with a simple, javascript-compatible array
Categories
(MailNews Core :: Backend, defect, P4)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b2
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file)
29.81 KB,
patch
|
Bienvenu
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•16 years ago
|
||
Opps. I meant to say: nsIMsgRecipientArray is only used in one interface for passing back a string array. In the function it is constructed, in the places where its called it is used, but not altered. Having a component all to itself is a bit excessive. In nsIMsgCompFields.idl, we should replace nsIMsgRecipientArray SplitRecipients(in AString aRecipients, in boolean aEmailAddressOnly); with: void splitRecipients(in AString aRecipients, in boolean aEmailAddressOnly, out unsigned long length, [array, count_is(length), retval] out AString recipients); and then we can drop nsIMsgRecipientArray.
Assignee | ||
Comment 2•16 years ago
|
||
(In reply to comment #1) > void splitRecipients(in AString aRecipients, in boolean aEmailAddressOnly, out > unsigned long length, [array, count_is(length), retval] out AString > recipients); actually, "out AString recipients" should be "out wstring aResult"
Assignee | ||
Comment 3•16 years ago
|
||
I have a WIP patch for this.
Assignee: nobody → bugzilla
Priority: -- → P4
Target Milestone: --- → Thunderbird 3.0b2
Assignee | ||
Comment 4•16 years ago
|
||
This drops nsIMsgRecipientArray replacing it as I previously suggested and doing the necessary changes around that. Also included a test case for splitRecipients so we know that is still working correctly.
Attachment #352109 -
Flags: superreview?(neil)
Attachment #352109 -
Flags: review?(bienvenu)
Comment 5•16 years ago
|
||
Comment on attachment 352109 [details] [diff] [review] The fix and test >+ /* nsMsgRecipientArray* pAddrArray = new nsMsgRecipientArray; > if (! pAddrArray) >+ return NS_ERROR_OUT_OF_MEMORY;*/ >+ >+ // rv = pAddrArray->QueryInterface(NS_GET_IID(nsIMsgRecipientArray), (void **)_retval); >+ // if (NS_SUCCEEDED(rv)) >+ // { ;-) >+ nsCAutoString recipientsStr; >+ char * names; >+ char * addresses; >+ PRUint32 numAddresses; >+ >+ CopyUTF16toUTF8(aRecipients, recipientsStr); >+ >+ rv = parser->ParseHeaderAddresses(recipientsStr.get(), &names, >+ &addresses, &numAddresses); Can we possibly use NS_ConvertUTF16toUTF8(aRecipients).get() here? Failing that, NS_ConvertUTF16toUTF8 recipientsStr(aRecipients); >+ PRUnichar** result = (PRUnichar**) PR_MALLOC(sizeof(PRUnichar*) * numAddresses); NS_Alloc >+ result = nsnull; Nit: don't need this, it's not used again.
Attachment #352109 -
Flags: superreview?(neil) → superreview+
Comment 6•16 years ago
|
||
By the way you removed 16 of the 24 whitespace errors.
Comment 7•16 years ago
|
||
Comment on attachment 352109 [details] [diff] [review] The fix and test very nice. In addition to Neil's nits, - recipient = inputArray.StringAt(index) + recipient = inputArray[index] I know you don't need a ; here, and you were just changing an existing line, but one would be nice :-) spaces around the '=' + PRUint32 i=0;
Updated•16 years ago
|
Attachment #352109 -
Flags: review?(bienvenu) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Pushed with comments fixed: http://hg.mozilla.org/comm-central/rev/43cd29a7832d
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•