79885 - LDAP autocomplete doesn't show pretty name

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P2)

Description

[Hong Kwon]

LDAP autocomplete/autocompletion doesn't show the pretty name when doing
typedown.  This makes the feature difficult to use.  Consider typing in the name
"Steve" - instead of being able to find the Steve you want you merely see a list
of valid Steve email addresses.  For example:

[e-mail]
[e-mail]
[e-mail]
[e-mail]

Something much more useable would be the following types of results:

[e-mail] <Steve Chang>
[e-mail] <Steven C. Murray>
[e-mail] <Steve Roswell>
[e-mail] <Stephen Doe>


Steps to reproduce
1. Set up LDAP typedown addressing to point to your LDAP server
2. Open mail compose.
3. Type in a few characters.. e.g. 'ste'

Expected Results:

Autocomplete results in the form:
[e-mail] <real name>

Actual Results:

Autocomplete results in the form:
[e-mail]

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

*** Bug 77384 has been marked as a duplicate of this bug. ***

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

For this to work, nsLDAPAutoCompleteSession.cpp needs to be able to parse it's
outputFormat attribute, which is on my plate.

Comment 3

[Marek Z. Jeziorek]

Setting target milestone to 0.9.2 (check it in anytime, even before, when the
tree is open for). Per PDT triage.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

OK, here's my current game plan for this: outputFormat will be specified as a
string, and inside that string, attributes will be quoted by a pair of |
characters (or maybe { } or whatever; suggestions invited). All other characters
other than the quote character(s) will be considered literals.

If the addressing widget finds a .autoComplete.outputFormat hidden pref for the
LDAP server in question, it'll pass that to the autocomplete session. Otherwise,
the LDAP session will use its internal default of:

|cn| <|mail|>

This allows for more flexibility than with the 4.5 ".autoComplete.attributes"
scheme (see <http://docs.iplanet.com/docs/manuals/communicator/ldap45.htm>),
because arbitrary text can be included. This may be handy once it's possible to
autocomplete from multiple directories, so that one can easily visually
differentiate between instances of identical names completed from different
directories, eg

John Smith (CompuServe) <[e-mail]>
John Smith (Beer Googles) <[e-mail]>

Still need to figure out whether this means I should store outputFormat as a
CharPref or UnicodePref; also need to check the mail RFCs to understand what
sort of i18n info is allowed in addrspecs (ie From, To, CC, ...).

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

I talked this over with Leif last night, and he pointed out that | could be a
bit confusing, since it already has special semantics in LDAP search filters.

Additionally, I grovelled through RFC 2822 for a while, and found that if you
quote it, pretty much anything (US-ASCII at the very least) can be in these headers.

So, new theory goes like this: attributes are delimited by curly braces, and \
can be used to escape (ie make literal) anything including itself.  The default,
then, looks like this:

cn <{mail}>

I'll probably be implementing today, so if you've got any comments, now would be
a great time to chime in.  :-)

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

I meant 

{cn} <{mail}>

of course.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch, v1

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

I've attached a basic patch that does this.  I still need to do a certain amount
of cleanup before it's ready to be reviewed, but it's a start.  I think I may
need to introduce another type of attribute; perhaps [foo].  This would mean
that the token is required for an entry; {foo} would mean that it's optional. 
So the default string would become:

{cn} <[mail]>

Since not having a cn (pretty name) is no big deal, but not having an email addr
makes an entry not worth displaying in the UI.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v2

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

OK, I think this is ready for review; any volunteers?  :-)

Here's the documentation for outputFormat, cribbed from the interface file.

     * Required attributes are delimited by curly braces, and optional 
     * attributes are determined by brackets.  Backslash escapes any 
     * character, including itself.  The default template is
     * "[cn] <{mail}>", without the quotes.  This will generate autocomplete
     * items of the form "John Doe <[e-mail]>", or, if the cn is not
     * found, " <[e-mail]>".  This latter form is suboptimal, in that
     * the space and angle brackets are a bit ugly, but should work.  See
     * bug 81961.

Comment 11

[Jean-Francois Ducarroz]

looks good. R=ducarroz

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch2, version 1

Comment 13

[Dan Mosedale (:dmosedale, :dmose)]

Doh!  Forgot to attach the second part of the patch, which is a minor tweak to
the JS in the compose window.

Comment 14

[Jean-Francois Ducarroz]

R=ducarroz for patch 2

Comment 15

[Scott MacGregor]

I reviewed 05/21/01 11:53 and 05/21/01 17:42. If those are the only patches you
needed a super review on, then they both looked correct and safe to me. sr=mscott

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Yup, those were the two.  Thanks!

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

a=dbaron for trunk checkin

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

Fix checked in.

Comment 19

[yulian chang]

2001061304 winds trunk build
Verified