177968 - Screen names do not get imported (from ldif files)

Status: VERIFIED FIXED

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

Description

[marina]

*** observed with 10-31-1.0 build ***
Steps to reproduce:
- export the PAB in .ldif format (have some card with screen names in there);
- import the Address Book back;
// note that Screen names are missing from the imported Address Book

Comment 1

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

mass re-assign.

Comment 2

[Mark Banner (:standard8)]

This also happens for csv files.

Comment 3

[Mark Banner (:standard8)]

csv import/export of screen name fields covered by bug 261882. Importing of
screen names needs small fix to ldif import (recongnise the screen name field)
once bug 119948 has it's first patch in. Therefore taking in advance

Comment 4

[Mark Banner (:standard8)]

Attachment: Allow import of screen name from ldif files

Now that we don't have as much duplicated ldif code, here's the patch to fix
the importing of the screenname (we already export it). works for both tb & sm.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 188225 [details] [diff] [review]
Allow import of screen name from ldif files

Turns out that the only use of "mozilla_aimscreenname" in the tree, according
to lxr, is in a comment in nsIAddrDatabase.idl.  The addressbook code itself
uses "nscpaimscreenname", so let's stick with that until the LDIF schema
changes land.

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> (From update of attachment 188225 [details] [diff] [review] [edit])
> Turns out that the only use of "mozilla_aimscreenname" in the tree, according
> to lxr, is in a comment in nsIAddrDatabase.idl.  The addressbook code itself
> uses "nscpaimscreenname", so let's stick with that until the LDIF schema
> changes land. 

Our export table (for ldif) uses _aimscreenname with the mozilla prefixed, see 
http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAddressBook.cpp#
124
That's why I used it here. Though we perhaps should also use nscpaimscreenname?

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 188225 [details] [diff] [review]
Allow import of screen name from ldif files

As discussed on IRC see my previous comment on this bug. Requesting review
again for decision later.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 188225 [details] [diff] [review]
Allow import of screen name from ldif files

I'd like to propose that any patch that touches the attribute here should at
least make us internally consistent.  Since nscpaimscreenname has been used in
the LDAP code for a long time, it seems reasonably likely to match what's
deployed in the real world.  How about we change all instances to that?

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Although since importing can accept both versions, maybe the best thing to do is
make a patch that allows both.

Comment 10

[Mark Banner (:standard8)]

Attachment: Allow import for nscpaimscreenname and mozilla_aimscreenname (checked in)

This patch allows importing of both nscpaimscreenname and mozilla_aimscreenname
fields.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 188855 [details] [diff] [review]
Allow import for nscpaimscreenname and mozilla_aimscreenname (checked in)

r+sr=dmose

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 188855 [details] [diff] [review]
Allow import for nscpaimscreenname and mozilla_aimscreenname (checked in)

Requesting approval on minor change to import ldif files - allow us to import
screen name which we currently do export via ldif. Low risk, affects sm and tb.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 188855 [details] [diff] [review]
Allow import for nscpaimscreenname and mozilla_aimscreenname (checked in)

looks like 1.8b3/avairy1.1a2 have passed, requesting approval for 1.8b4, same
comments as previous apply.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 188855 [details] [diff] [review]
Allow import for nscpaimscreenname and mozilla_aimscreenname (checked in)

Checked in:
/cvsroot/mozilla/mailnews/addrbook/src/nsAbLDIFService.cpp,v  <-- 
nsAbLDIFService.cpp
new revision: 1.2; previous revision: 1.1

Comment 15

[Mark Banner (:standard8)]

Fix is in, the cvs/txt/tab case will be handled in bug 261882.

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; I checked both Netscape 7.1 and Mozilla SeaMonkey build
2005-07-15-05 on Windows XP.