Closed Bug 474439 Opened 11 years ago Closed 11 years ago

name incorrect with ldap lookup


(MailNews Core :: Address Book, defect, major)

Windows XP
Not set


(Not tracked)

Thunderbird 3.0b2


(Reporter: wsmwk, Assigned: standard8)



(Keywords: regression)


(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #473715 +++

name information is incorrect with ldap lookup
1. click ldap address book
2. search on address

contact results show only email address or first name

also regression of bug 419595?

20090118 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090119 Shredder/3.0b2pre
search itself consistently works OK - I get correct matches on firstname, lastname, email.
but search results have email address in the name fields, or first name only - which is incorrect or incomplete
the only good info displayed is email address and phone number.

20090110  works
20090111  works
20090112  works
20090113  ldap doesn't work at all (there was ldap bustage iirc)
20090115  fails
Severity: normal → major
View -> Show Name  is display name

in the badie, contact info displayed is email, title, phone
in the goodie, contact info displayed is fname, lname, addrline1, Notes (in addition to the above) - and display name is populated with fname+lname+notes

emailed ldap:5 log
I see this as well, only Firstname, email, and work phone are filled in lookup.

View -> Show Name as -> Display name:  Results in just the "name" part of "name", not ldap "cn" value.

View -> Show Name as -> First Last: Results in just firstname.

View -> Show Name as -> Last First: Results in just firstname.

This is in addition to ldap initialization error when typing a partial name from the address line of a new compose email.
Flags: blocking-thunderbird3?
Ok, I see the problem now. We have a set of preferences ldap_2.servers.default.* that are used to map one attribute to multiple possibilities on the server.

When one of these has greater than one attribute (i.e. a comma in it), I think we're not requesting the attributes from the server, hence we don't get anything to display.

This could be a regression from the recent password manager change, but I'm sort of thinking its more likely a ParseString regression (that also recently changed).
Assignee: nobody → bugzilla
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Target Milestone: --- → Thunderbird 3.0b2
(In reply to comment #1)
> 20090110  works
> 20090111  works
> 20090112  works
> 20090113  ldap doesn't work at all (there was ldap bustage iirc)
> 20090115  fails
These builds don't match up with the dates of last week's ParseString bustage.
Attached patch The fix (obsolete) — Splinter Review
It turns out the way that I had changed the nsAbLDAPAttributeMap::getAllCardAttributes function had caused nsLDAPURL to be upset - rather than giving it a list of comma separated attributes, we are now giving it an array of attributes, unfortunately some of those attributes are of the format "foo,bar" and I think nsLDAPURL/LDAP c-sdk is getting confused as to what is happening and isn't handling them properly.

This change just makes getAllCardAttributes return all attributes as individual items in the array which would be the expected functionality anyway. Note I had to add the extra String() because javascript wasn't identifying the prop items as string objects.
Attachment #360015 - Flags: superreview?(neil)
Attachment #360015 - Flags: review?(neil)
Blocks: 419595
Comment on attachment 360015 [details] [diff] [review]
The fix

>     for each (var prop in this.mPropertyMap) {
Bad naming. Since you're using for each, you should use var attrArray (c.f. checkState). Which explains why your attempt to use split failed ;-)
Attachment #360015 - Flags: superreview?(neil)
Attachment #360015 - Flags: superreview-
Attachment #360015 - Flags: review?(neil)
By the way, there's a logic bug that dates back to the original version :-(

nsAbLDAPAttributeMap contains objects in its prototype. These objects will get shared between its instances :-( [nsAbLDAPAttributeMapService is OK because it's a singleton.]
Attached patch The fix v2Splinter Review
Simpler fix plus creating objects in the constructor.
Attachment #360015 - Attachment is obsolete: true
Attachment #360019 - Flags: superreview?(neil)
Attachment #360019 - Flags: review?(neil)
Comment on attachment 360019 [details] [diff] [review]
The fix v2

>+      // Individual properties may have more than one attribute, so we
>+      // must split them here.
This comment no longer makes sense. r+sr=me with it removed.
Attachment #360019 - Flags: superreview?(neil)
Attachment #360019 - Flags: superreview+
Attachment #360019 - Flags: review?(neil)
Attachment #360019 - Flags: review+
Closed: 11 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090202 Shredder/3.0b2pre
You need to log in before you can comment on or make changes to this bug.