53216 - AB:Display names typed into Mailing list disappear after closing

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect, P2)

Description

[marina]

**** observed with 2000-09-19-09 build ****
Steps to reproduce:
- Edit existing Mailing List by typing into it new names (Display Name followed 
by email address for ex: alex smith <[e-mail]>);
- close Mailing list by OK'ing it;
- re-open it (or send Mail, get it and observe):
//note: all Display Names that you typed into the List are stripped off leaving 
the email addresses only

Comment 1

[nhottanscp]

Is this a regression?

Comment 2

[chuang]

The display name should be entered through New Card or Edit Card.  Mailing list 
dialog only get the email address from the mailing list.  It is not a 
regression.

Comment 3

[marina]

In that case we have to language it differently , now when when you open a 
Mailing List it says : type names or drag/drop addresses into the Mailing list 
below". Type names could be understood as a possibilty to enter Display names 
there

Comment 4

[Suresh]

Possibly a dupe of 46058?

Comment 5

[chuang]

Yes, basically is the same.  Mailing list dialog doesn't pasre the entered 
string to get the display name if there's one. 

Comment 6

[scottputterman]

reassigning to chuang.

Comment 7

[fenella]

QA-assign-to fenella.

Comment 8

[scottputterman]

reassigning to racham.

Comment 9

[Ninoschka Baca]

What is the solution for this bug? Are we going to allow Display Names when
creating a list or does the text need to change in the dialog?

Marking nsbeta1 because it should be more clear.

Comment 10

[Håkan Waara]

I definitely think you should be able to type in whatever name you want when 
creating or editing a mailing list. This is a bug and not a feature. :-)

Comment 11

[Ninoschka Baca]

Marking nsbeta1 so that the Display Name is retained.

Comment 12

[Samir Gehani]

Mail triage team: nsbeta1+/adt2

Comment 13

[Cavin Song]

Reassigning.

Comment 14

[Cavin Song]

Reassigning.

Comment 15

[shliang]

Attachment: patch

Comment 16

[Cavin Song]

Comment on attachment 113042 [details] [diff] [review]
patch

r=cavin.

Comment 17

[shliang]

Attachment: patch

Comment 18

[Cavin Song]

Comment on attachment 113255 [details] [diff] [review]
patch

r=cavin.

Comment 19

[(not reading, please use [e-mail] instead)]

Comment on attachment 113255 [details] [diff] [review]
patch

1)

+    nsXPIDLString name;
+    pCard->GetDisplayName(getter_Copies(name));
+    if (name) {
+      AddDisplayName(pCardRow, NS_ConvertUCS2toUTF8(name).get());
+      err = m_mdbPabTable->AddRow(GetEnv(), pCardRow);
+    }

Because name is a nsXPIDLString, I think you want:

if (!name.IsEmpty()) {

instead of 

if (name) {

2)

What happens if the user add a display name to an email address that was
already in the list?
(does your code do the right thing?)

3)

+	   if (beginpos != 0) {
+	     var name = fieldValue.slice(0, beginpos - 1);
+	     cardproperty.displayName = name;
+	   }

actually, the whole "if (cardproperty) {}" block (which you are adding to) has
a problem.

we are parsing the line by hand, and we need to use the header parser.	

(there can be escaped characters and other things that simple parsing won't
catch.)

here's the start of what I think we should do:

var gMsgHeaderParser =
Components.classes["@mozilla.org/messenger/headerparser;1"].getService(Componen
ts.interfaces.nsIMsgHeaderParser);

...

if (cardproperty)
{
  var numAddresses = gMsgHeaderParser.parseHeadersWithArray(emailAddresses,
addresses, names, fullNames);
  var addresses = {};
  var fullNames = {};	
  var names = {};
  var numAddresses = msgHeaderParser.parseHeadersWithArray(fieldValue,
addresses, names, fullNames);
  // numAddresses should always be 1, right?
  cardproperty.primaryEmail = addresses.value[0];
  cardproperty.displayName = names.value[0];

  if (doAdd || (doAdd == false && pos >= oldTotal))
    mailList.addressLists.AppendElement(cardproperty);
  pos++;
}

can you try that out?  
numAddresses should be 1, but might not be if the line is blank 
or if the user does something like "[e-mail],[e-mail]" on a line.

Can you investigate?

Comment 20

[shliang]

Attachment: patch

2. What happens if the user add a display name to an email address that was
already in the list? (does your code do the right thing?)
yes this works.

3. using the msg header parser now, in most cases numAddresses is 1, otherwise
only the first one entered in the line is added.

Comment 21

[(not reading, please use [e-mail] instead)]

Comment on attachment 113635 [details] [diff] [review]
patch

sr=sspitzer, thanks shuehan

Comment 22

[shliang]

marking fixed.

Comment 23

[Ninoschka Baca]

Trunk build 2003-03-03: WinXP, Linux RH 8

These are the results so far. I'm not sure If this is what is expected:

1. In a list dialog type a display name followed by an email address and it is
retained after closing/reopening the list (i.e. John Smith <[e-mail]>)

2a. In a list dialog type two address one one line, separated by a comma and
only the first address remains (i.e. [e-mail], [e-mail] --> results in
[e-mail] remaining but [e-mail] is gone)

2b. Performed the same test as 2a except these entries include the display name
and only the first display name/email address is retained (i.e. John Smith
<[e-mail]>, John Smith2 <[e-mail]> --> results in John Smith
<[e-mail]> remaining but John Smith2 <[e-mail]> is gone)

Comment 24

[Ninoschka Baca]

Verified Fixed. I checked with shliang and this is the expected behavior.