263304 - The Address Book doesn't show the LDAP-field "labeledURI" as Website

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Volker Grabsch]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de-DE; rv:1.7) Gecko/20040917 Firefox/0.9.3
Build Identifier: Mozilla Thunderbird 0.8 (20040913)

I have an LDAP server which contains some "inetOrgPerson" records. When I look
at them using Mozilla Thunderbird's Address Book it doesn't show die LDAP-field
"labeledURI". There's nothing shown in the field 'Website' in the Address Card.

Reproducible: Always
Steps to Reproduce:
1. Create new Directory Server entry to an LDAP server which contains
inetOrgPerson entries
2. Search for one of these entries where the field "labeledURI" is set
3. Open the entry
4. Click on "Address"
Actual Results:  
The two "Website" fields are empty.

Expected Results:  
The second "Website" field (the every last text field) should contain the value
of the field "labeledURI".

The schema "inetOrgPerson" as of RFC 2798 contains a field "labeledURI". Since
Thunderbird respects the other fields (cn, sn, givenName, st, title, ...) it
should respect the field "labeledURI" as well.

Comment 1

[Doug Wright]

-> Core:MailNews:LDAP Integration

Comment 2

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 3

[Gervase Markham [:gerv]]

This bug has been automatically resolved after a period of inactivity (see above
comment). If anyone thinks this is incorrect, they should feel free to reopen it.

Comment 4

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

sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.

Comment 5

[Josh Geenen (:pi)]

I can confirm this behavior using an LDAP server and a record that conforms to the inetOrgPerson schema.  There is more information about the labeledURI attribute in RFC 2079:
http://www.rfc-archive.org/getrfc.php?rfc=2079

Basically, it contains a URI and, optionally, a label separated from the URI by one or more spaces.

Before submitting a patch, should this URI be set as the work or home webpage?  I am leaning toward the work webpage, but either one would make sense.  And then, in which order should the other attributes for the URL appear in the default attribute map?
For example (mailnews/mailnews.js): pref("ldap_2.servers.default.attrmap.WebPage1", "mozillaWorkUrl,workurl,labeledURI");

My current patch removes the label because, if the label is kept, the link does not work as expected when viewing the card.

Assigning the bug to myself and marking as NEW...

Comment 6

[Josh Geenen (:pi)]

Attachment: A patch for LDAP and LDIF imports

This patch adds labeledURI to the LDAP attribute map and LDIF imports under the Work URL and strips the label.  It worked for a basic LDIF import and on my local LDAP setup with and without a label.

I can write a quick unit test if this is worth fixing.

Comment 7

[Volker Grabsch]

Since the current implementation violates the LDAP standard, I do think this is "worth fixing".

It would be great if there was at least one free mail client which doesn't require nasty quirks to work with a totally standard conforming LDAP address book.

Comment 8

[Ludovic Hirlimann [:Usul]]

(In reply to comment #7)
> Since the current implementation violates the LDAP standard, I do think this is
> "worth fixing".
> 
WONTFIX ?

Comment 9

[Volker Grabsch]

(In reply to comment #8)
> (In reply to comment #7)
> > Since the current implementation violates the LDAP standard, I do think this is
> > "worth fixing".
> > 
> WONTFIX ?

No, that's not what I meant. To make it more clear:

The current implementation which ignores the labeledUri attribute violates the LDAP standard, so I would really love to see this fixed. The proposed patch is a step into the right direction. I think that it is worth the work to add additional unit tests and to include the patch.

Comment 10

[Josh Geenen (:pi)]

Attachment: Updated path and unit test

This is an updated patch and unit test.  The unit test fails before the patch and passes after, as expected.

Comment 11

[Josh Geenen (:pi)]

Attachment: Patch and unit test

My last attachment did not contain the complete unit test (I forgot to include a the new LDIF used in the import).

The test passes in Windows and Linux with debugging disabled.

Comment 12

[Ludovic Hirlimann [:Usul]]

Josh you needs to specify who should do the review. I don't know who does it for LDAP so I've cced a few persons that know better.

Comment 13

[Josh Geenen (:pi)]

Comment on attachment 382073 [details] [diff] [review]
Patch and unit test

Let me know if this review should go to someone else.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 382073 [details] [diff] [review]
Patch and unit test

>diff -r 120b7786e2b2 mailnews/import/test/unit/resources/bug_263304.ldif

>+labeledURI: http://127.0.0.1

This should have a test label on the end, otherwise you're not actually testing the bug.

r=Standard8 with that fixed.

Comment 15

[Josh Geenen (:pi)]

Attachment: Patch and unit test v2

Adds a label to the labeledURI in the imported LDIF file as requested.

Comment 16

[David :Bienvenu]

Comment on attachment 389607 [details] [diff] [review]
Patch and unit test v2

use -1 instead of kNotFound here, since kNotFound should be going away:

+      if (index != kNotFound)
+        column.Truncate(index);

Comment 17

[Josh Geenen (:pi)]

Attachment: Patch & unit test for checkin

This patch changes the kNotFound to -1.

Thanks for the r and sr.

Comment 18

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/f7e7acfc8080