76593 - add param for configuring minimal length of strings to search

Status: VERIFIED FIXED

(Directory Graveyard :: LDAP XPCOM SDK, defect, P2)

Description

[Dan Mosedale (:dmosedale, :dmose)]

From Leif's comments in 70933:

* As mentioned before, I still believe we need to be careful when we
autocomplete - if a user types just one or two characters ( "j" / "jo"), that
could quite likely generate LDAP searches that are unindex on many LDAP servers.
iPlanet's LDAP servers would most certainly treat such searches (e.g. "cn=j*")
as an unindexed search, since it only index substrings that are 3 characters or
longer.

This should be an attribute on nsILDAPAutoCompleteSession, since different folks
may want this set differently.

Comment 1

[yulian chang]

reassign to Olga as QA contact

Comment 2

[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 3

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: A patch to fix this, including a new hidden pref.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Short patch attached; review requested.

Comment 5

[Jean-Francois Ducarroz]

Looks good. R=ducarroz

Comment 6

[Leif Hedstrom]

Looks good, two *very* nit-picky comments, that you can consider at your own
discretion:

> Strings shorter than this will return
>     * |nsIAutoCompleteStatus::ignored| rather than triggering a
>     * search.  This is desirable because many LDAP servers do not
>     * index strings that are too short, which means that searching
>     * them will be much less efficient.  If unset, defaults to 3, 
>     * since that's a popular LDAP server minimum indexing size.


You might want to explain that "searching" here really means "substring
searching". I.e. this really only is an issue if you have wildcards in a search
(e.g. cn=*leif*), since some LDAP servers will not index substrings that are
shorter than 3 characters. You might also want to mention the behaviour of
"anchoring" the search, e.g. "cn=le*", which would be an indexed search).


Also, would it be more readable to #define a constant like
|kDefaultMinStringLength| with the default "3" somewhere in the beginning of the
|.cpp| or |.h| file?


So, R=leif

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v2

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v2

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

OK, patch v2 (mozilla hung, which is why it got attached twice) is here.  It has
three changes:

* instead of using ${directoryServer}.minStringLength, changed it to
${directoryServer}.autoComplete.minStringLength, since this parameter doesn't
apply to all searches using this directory server

* made a small optimization suggested by leif: make sure that mMinStringLength
is non-zero before bothering to call nsCRT::strlen on the search string.

* after discussion with Leif, changed the default from a 3 character minimum
to 0 (no minimum).  This is because the deployed base of autocomplete users
today already has directory servers which can handle the entire range of
searches (since that's what 4.x does), performance in mozilla even in big
searches is reasonable, and having any limit at all is somewhat unintuitive from
a naive user's point of view, since there are a non-trivial number of people
with (eg) two character last names.
 

Comment 10

[Srilatha Moturi]

r=srilatha on the diffs for MsgComposeCommands.js 

Comment 11

[David :Bienvenu]

sr=bienvenu

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Patch checked in,

Comment 13

[olga charapaev]

minStringLength works, hidden pref works
Verified on WinNT 4.0 , MacOS 9.1, Linux RH7.1 with 2001053004, 2001060611 builds.

Setting minStringLength  to 0 (by default) though causes a hang  when we 
search with just one char entered. I'll open a new bug on this with more comments.

Marking this bug as verified.