If you create a new mailing list and start using type ahead to add entries to the list, we don't use the LDAP directory if one has been configured for type ahead here. Apparently 4.x did that. I guess we would also have to create a new card for each LDAP entry that got added to the list?

taking - yes, we would have to add a card in the main ab if an ldap entry was accepted in the list.

this uses the default ldap auto complete server for autocomplete in the mailing list UI.

Comment on attachment 137516 [details] [diff] [review] proposed fix One thing I'm not happy about is this pretty much clones the setupLdapAutocompleteSession() routine from msgComposeCommands.js - if I wanted to try to share this code, where would I put it? It's slightly different, but I think the differences could be reconciled.

may not be ideal but in tbird we have a file called mailCore.js which contains JS shared by all 3 components (ab,compose,3-pane).

Has the proposed fix fallen on the trunk? It's an issue for us here at Georgetown U., where Mozilla replaced NS Communicator 4.x as our recommended e-mail client, and users need addresses from LDAP for their mailing lists.

No, it has not landed on the trunk. It will land when the tree opens after 1.7a. For the trunk, I'd like to share the code instead of duping it.

I see this is still in the 'proposed fix' stage, but I thought it would be in 1.7. Will it make 1.8?

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

larry wrote "Will it make 1.8?" :) apparently not

What is up with this bug? This proposed patch has been in the queue for almost a year and a half, and my users are complaining about this. Does Mozilla intend to penetrate the enterprise space or not? If so, this is an important feature!

I also forgot to mention that Thunderbird 1.x also exhibits the same behavior.

(In reply to comment #11) > I also forgot to mention that Thunderbird 1.x also exhibits the same behavior. TB equivalent is Bug 282040 which has a proposed patch that hasn't yet landed. The last bug entry states "... possibly - I'll land it when the tree opens post 1.1a in a week or so."

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

David, is the patch on this bug still valid? The checkin for bug 282040 (thunderbird) is slightly different. We'd like to get the fix for this into the suite, I'm happy to port the patch from 282040 if you are too busy.

Mark, I don't know if it's still valid for the trunk, but either it, or the one I checked into thunderbird should work. Neil was looking at the patch I checked in, with an eye towards figuring it out for Seamonkey, including avoiding the duplicated code, so he might have an idea.

This is the suite version of the tb patch from bug 282040. It works on my build, but I haven't got an LDAP server, so if someone could test it, I'd be grateful.

Yes it works on a LDAP server too, but it's hard to tell where it is picking the addresses up from because there is no icon to indicate if each address is from LDAP or other addressbook.

Comment on attachment 137516 [details] [diff] [review] proposed fix Cancelling prior review requests. David - if it's ok with you we'll take the new patch that I posted that is a variant of the thunderbird one. It should be more consistent that way.

This revised patch adds some changes to the themes so that we get the icons in the mailing list autocomplete that indicate where the address comes from. I've looked through the new autocomplete code and not found any changes that need making.

Yes tested it works on a LDAP server too and icons now indicate the source of the address.

Comment on attachment 183846 [details] [diff] [review] Suite patch v2 Requesting review for Mailnews parts (Neil can do the themes part if he does sr).

So this patch just copy-pastes a big chunk of code from the compose-window. Instead of copy/pasting, the code from the compose window should be factored out somewhere common so that it can be shared.

Comment on attachment 183846 [details] [diff] [review] Suite patch v2 Please follow dmose's advice in comment #22 (and ask him for review later - I'm really not of much help here).

(In reply to comment #17) > Yes it works on a LDAP server too, but it's hard to tell where it is picking the > addresses up from because there is no icon to indicate if each address is from > LDAP or other addressbook. Are Bug 118451 and Bug 191355 related ?

I would say not, but I could be wrong.

(In reply to comment #3) > (From update of attachment 137516 [details] [diff] [review] [edit]) > One thing I'm not happy about is this pretty much clones the > setupLdapAutocompleteSession() routine from msgComposeCommands.js - if I wanted > to try to share this code, where would I put it? It's slightly different, but I > think the differences could be reconciled. > How about in a JS component in mailnews/addrbook/src? Conceptually, from the point of view of mail/news, LDAP is just another addressbook, and over the long haul, it would be nice to try and coalesce the code in that direction.

(In reply to comment #26) > (In reply to comment #3) > > (From update of attachment 137516 [details] [diff] [review] [edit] [edit]) > > One thing I'm not happy about is this pretty much clones the > > setupLdapAutocompleteSession() routine from msgComposeCommands.js - if I wanted > > to try to share this code, where would I put it? It's slightly different, but I > > think the differences could be reconciled. > > > On second thought, mscott's suggestion in comment 4 sounds better. There's no real reason to involve extra XPCOM abstraction here. > How about in a JS component in mailnews/addrbook/src? Conceptually, from the > point of view of mail/news, LDAP is just another addressbook, and over the long > haul, it would be nice to try and coalesce the code in that direction. > >

(In reply to comment #3) >(From update of attachment 137516 [details] [diff] [review]) >One thing I'm not happy about is this pretty much clones the >setupLdapAutocompleteSession() routine from msgComposeCommands.js >- if I wanted to try to share this code, where would I put it? I was thinking that it could be put in addressingWidgetOverlay.js

(In reply to comment #28) > I was thinking that it could be put in addressingWidgetOverlay.js The suite patch v2 does put the function in addressingWidgetOverlay.js, however I *think* Dan was referering to the fact that by copying the function, tb and seamonkey will have seperate copies of the same function. Dan - is that right? or did you think we would end up with two copies of it in mailnews or mail? AFAIK, there's currently not really any sensible js files (relating to address books/mail compose areas) that both seamonkey & tb use to share this function between, so we would have to create a new js file and move the function from the one in mail to mailnews. I don't know if Scott will be happy with that.

I'm not objecting to the fact that this function is being forked between tb & sm, just like much of the mail front-end has. I'm objecting to the fact that it's already been forked inside tbird (MsgComposeCommands.js & abCommon.js) and that this patch proposes to do a similar fork inside seamonkey (MsgComposeCommands.js & addressingWidgetOverlay.js).

This seems to work for me, but only on the very first entry ... digging through these unassigned bugs to try and find a solution.

Probably fixed by Bug 452232 - Move LDAP autocomplete over to toolkit interfaces