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)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

      No description provided.
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.
Blocks: 466613
(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"
I have a WIP patch for this.
Assignee: nobody → bugzilla
Priority: -- → P4
Target Milestone: --- → Thunderbird 3.0b2
Attached patch The fix and testSplinter Review
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 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+
By the way you removed 16 of the 24 whitespace errors.
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;
Attachment #352109 - Flags: review?(bienvenu) → review+
Pushed with comments fixed: http://hg.mozilla.org/comm-central/rev/43cd29a7832d
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 468807
You need to log in before you can comment on or make changes to this bug.