139786 - Some LDAP attributes not returned in third party search

Status: RESOLVED FIXED

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

Description

[John Marmion]

OpenOffice.org(OOo) integrates with Mozilla's LDAP Address Book implementation.
OOo LDAP Query does not request the "card:nsIAbCard" but instead rqeuests a list
of attributes. The reason for bug #124022 was to request only the actual LDAP
attributes that Mozilla's supports. But a side effect of that change was the
list of attributes to be requested was created in reverse order than they appear
in the table on nsAbLDAPProperties.cpp. Thus e.g. a request to a Server which
supports "surname" and "sn" will return "surname" as it is requested first. But
mapping the Mozilla Attribute "LastName" to its corresponding LDAP attribute
will return "sn" as it appears first in the hash table. Thus no value will be
returned in this case as we have a misleading mismatch when comparing "surname"
and "sn". The proposed change does not effect the Mozilla LDAP functionality.

The problems associated with multiple LDAP attributes mapped to a single Mozilla
attribute was also raised in bug #126749.

Comment 1

[John Marmion]

Attachment: simple patch to address this failure

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 80781 [details] [diff] [review]
simple patch to address this failure

r=dmose

Comment 3

[John Marmion]

Attachment: keep patch up-to-date against the trunk

no change in content but patch offsets are wrong.

Comment 4

[John Marmion]

Attachment: Keep patch up-to-date against the Head

I will chase after an "sr" for this patch again after keeping the patch
up-to-date against the head.

Comment 5

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

Comment on attachment 94921 [details] [diff] [review]
Keep patch up-to-date against the Head

after this change, p is only used one place.

so you can change:

const MozillaLdapPropertyRelation* p =
MozillaLdapPropertyRelator::findLdapPropertyFromMozilla(n.get());

if (!p)
  continue;

to be:

// check if we support this property
if (!MozillaLdapPropertyRelator::findLdapPropertyFromMozilla(n.get()))
  continue;

since your are changing this code, can you take a minute and rename "n" to
something that's more meaningful?  There are two places in this file where we
are doing

nsCAutoString n;

Comment 6

[John Marmion]

Attachment: Update patch as requested by review

Patch updated as requested and use "nsCAutoString propertyName" rather than
"n".

Comment 7

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

Comment on attachment 95084 [details] [diff] [review]
Update patch as requested by review

sr=sspitzer

thanks for making the change.

I didn't see the other
"const MozillaLdapPropertyRelation* p".  Can you change that (like you did for
"n")  before checking in?  no need for a new patch, unless dmose requires it.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

I don't need to see another patch.  John, should this bug be re-assigned to you?

Comment 9

[John Marmion]

Attachment: Update patch as requested by 'sr'

Thanks guys. I will take ownership of this and check this in.

Comment 10

[John Marmion]

Assuming ownership of this bug.

Comment 11

[John Marmion]

Fix checked in. marking as Fixed.