102767 - Show Name As doesn't reflect to second line in Lower pane for card

Status: VERIFIED FIXED

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

Description

[Kasumi]

Comment 1

[Kasumi]

Build ID = 2001-09-25-04-0.9.4
OS = MacOS X,Win XP Ja, Professional, Version 2002

1. Launch Navigator
2. Select Tasks/Mail & Newsgroups
3. Create a new card using the following information.
   First - A
   Last  - B
   Display - automaticalle A B
4. Hilight this new card and take a look lower pane
   You will find A B in second line on gray bar.
5. Select View/Show Name as
   You will find checked Display Name
6. Change check to Last,First
7. Hilight this new card again and take a look lower pane
   You will find still A B in second line on gray bar.
   
It is verry important for Japanese.
Japanese never use order of First Name-Last Name.  
 

 

Comment 2

[Kasumi]

Attachment: detail

Comment 3

[scottputterman]

reassigning to racham

Comment 4

[ji]

Same problem for ASCII addressbook cards. Removed intl keyword.

Comment 5

[racham]

Address book UI bugs to me ([e-mail]). Reassigning this one back to
myself.

Comment 6

[scottputterman]

I think this is a dup, but I'm having trouble finding it.

Comment 7

[racham]

Attachment: patch, v1

Comment 8

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

your patch has a problem.  what if there is no first name or no last name?

Comment 9

[racham]

The block I have modified is in the scope of requirement of having those 2 names 
available
[if ( card.lastName && card.firstName )]

Suppose if first name or last name ot both didn't exist, today we display 
"Name:" in the name header row 
(http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/resources/content/abC
ardViewOverlay.js#141). 

Jennifer, I looked at the spec. I haven't noticed any statement that suggested 
us to display "Name:" when first or last name or both are not found. Let me if 
it has been decided that way and that decision not made it's way into the spec. 
I believe display "Name:" would probably suggest the user that this row is 
reserved for name and one/both name fields for this card is/are empty.

Here is what I found in the spec 
(http://mozilla.org/mailnews/specs/addressbook/):
<--
The "Card for <Name>" should match the name format (First Last, Last/First, 
Display Name) displayed in the "Name" column of the Results Pane. The
Name displayed in the title header right below the "Card for < >" should be 
First Name/Last Name (or Last First, based on internationalization
preference).
-->

We do the right thing with "Card for <name>". 
With the name displayed below that is the current problem. I fixed it to change 
depending on the selection made under "View -->Show Name as-->". One thing I 
need to take however though is when user decides display mode to be "Display 
Name". Today, with the patch, I display "<First Name> <Last Name>". From 
Kasumi's statement, it seems to be pretty important to default to "<Last Name>, 
<First Name>" for i18n cases. So, I can make that to be the choice for name 
displayed in the title header (i.e., below "Card for <name>").

Comment 10

[racham]

Let me know if you need me to post screen shots.

Adding Seth & JFD to the cc list.

Comment 11

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

old code:

if (last & first) {
  // set the seperator
}
if (lastFirst)
  // build name
else
  // build name

the new way, after you patch:


if (last & first) {
  if (lastFirst) {
    // get sep
    // build name
  }
  else {
    // get sep
    // build name
  }
}

that's not the same, right?  in the old way, we'd still show the name, but 
since we don;t have first and last, we don't define a seperator.

or am I confused?

Comment 12

[racham]

Seth, You are right.

Sorry about the confusion. I can't optimize as I expected here.

I will post a new patch that does ->

1. If "View -->Show Name as-->" choice is "First Last", then pick "<First Name>
<Last Name>" as the name title header.
2. If "View -->Show Name as-->" choice is "Last, First" OR "Display Name", then
pick "<Last Name>, <First Name>" as the name title header.

If any one has any objections, please update the bug.

Comment 13

[racham]

Attachment: patch, v2

Comment 14

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

I don't think you're handling case 0 properly.

from mailnews.js:  //0=displayname, 1=lastname first, 2=firstname first

something like this:

const kDisplayName = 0;
const kLastNameFirst = 1;
const kFirstNameFirst = 2;

switch (cvPrefs.nameColumn) { 
  case kFirstNameFirst:
    name = card.firstName + separator + card.lastName;
    break;
  case kLastNameFirst: 
    name = card.lastName + separator + card.firstName;
    break;
  case kDisplayName:
  default:
    name = card.displayName;
    break;
}
 

Comment 15

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

if you define those consts, we should use them in the switch 
(cvPrefs.nameColumn) statement in addressbook.js 

Comment 16

[racham]

Attachment: patch, v3 - Incorporated Seth's suggestions

Comment 17

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

looks good, except for this:

+ if ( cvPrefs.nameColumn == 2 ) {

should be

+ if ( cvPrefs.nameColumn == kFirstNameFirst ) {

make that change, and sr=sspitzer

Comment 18

[Jean-Francois Ducarroz]

Comment on attachment 55256 [details] [diff] [review]
patch, v3 - Incorporated Seth's suggestions

If you really incorporate Seth's suggestion: R=ducarroz

Comment 19

[racham]

Fix checked in after incorporating the last suggestion.

Marking Fixed.

Comment 20

[Ninoschka Baca]

Trunk build 2001-11-13-03:WinMe
Trunk build 2001-11-09: Linux RH 7.1, Mac 9.1

1. Viewing Last/First displays correctly in results pane and card pane

Question: What if I edit the card. Currently the title states
"Card for First Last". Shouldn't this dialog also follow the same Last/First 
format so it would state "Card for Last, First"?

Comment 21

[Kasumi]

I don't think so.
As you told thatresults pane and card pane are fixed, I think it is good enough. 

 

Comment 22

[Ninoschka Baca]

Verified Fixed.