232453 - ldap quick search and advanced search does not honor objectclass filters

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Scott MacGregor]

"Under Address Book, the address book filters don't work. You can create a new
LDAP address book, but the filter you add to limit the search does not work.
Regardless of what filter you enter, it does a search against objectlass=*
instead of against the subset of results you input into the "Search filter" box
under Address Book->Properties->Advanced."

Looks like this works for mail compose auto complete, we just aren't honoring
the filter info for address book searching. So hopefully the fix is pretty
straightforward.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

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

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

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

Comment 3

[Scott MacGregor]

See: 

http://lxr.mozilla.org/seamonkey/source/xpfe/components/autocomplete/src/nsLDAPAutoCompleteSession.cpp#68

and

http://lxr.mozilla.org/mozilla/source/mailnews/compose/resources/content/MsgComposeCommands.js#921

for pointers into how the filter works / is applied to autocomplete

Comment 4

[Scott MacGregor]

When the user changes the filter through the advanced properties dialog, we show
that in prefs.js by changing the .uri pref, appending the filter criteria at the
end:

i.e.

"ldap://directory.washington.edu:389/o=University of Washington,
c=US??sub?(givenName=a*)"

Comment 5

[Scott MacGregor]

Attachment: first cut at a fix

This patch modifies our directory query to always AND the default search filter

for the server with the filter we generate based on what the user is searching
for.

I'm still trying to test this out.

Also, there is a method in the LDAP SDK called CreateFilter:
http://lxr.mozilla.org/seamonkey/source/directory/xpcom/base/public/nsILDAPService.idl#182


which might do some of the merging of search terms for me, but I got the
impression using it would be overkill for the type of merging we are doing
(just joining the two strings with an AND operator.

Comment 6

[Scott MacGregor]

Comment on attachment 140110 [details] [diff] [review]
first cut at a fix

this patch appears to be working. Submitting for review. I'm taking the search
term from the user and then joining it with an AND operator with the search
filter terms set up on the ldap directory properties tab.

Comment 7

[David :Bienvenu]

Comment on attachment 140110 [details] [diff] [review]
first cut at a fix

ok by me if it's ok with Dan.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 140110 [details] [diff] [review]
first cut at a fix

Despite the copied comment, there's no test here for whether urlFilter is
empty, and I think there needs to be.  Looks like this bug was introduced
introduced in version 1.34 of nsLDAPAutoCompleteSession.  So fix that here (and
there, if you like :-), and r=dmose.

(or in the nsLDAPAutoComplte

Comment 9

[Scott MacGregor]

Attachment: updated patch with comments from dmose

Comment 10

[Scott MacGregor]

fixed on the trunk and the m4 branch