125197 - ab advanced search: name searches: should it be displayname, first name and last name?

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Description

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

name searches:  should it be displayname, first name and last name?

right now, it is just display name.

I think it should be an "or" search of display, first and last.

Comment 1

[Ninoschka Baca]

Marking nsbeta1. I would expect Advance Search to also support First Name and 
Last Name, just as Quick Search does. 

I don't know how common it is to have a Display Name that is different from the 
First and Last Name. 

Comment 2

[jglick]

>I think it should be an "or" search of display, first and last.

Agree.

Comment 3

[Ninoschka Baca]

Marking nsbeta1.

Comment 4

[Ninoschka Baca]

Could Nick Name also be added to an "any name" search?

Comment 5

[Samir Gehani]

Mail triage team: nsbeta1+/adt2

Comment 6

[Cavin Song]

Reassigning.

Comment 7

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

shuehan pointed why this is a non-trivial problem.

the addressbook doesn't handle mixed and/or searches 

(note, mscott made it so mail search does for mail views).

here's the code comment I added when I added the "any name" and "any number"
thing to the trunk on 9/11/2002.

from onSearch() in ABSearchDialog.js:

     // currently, we can't do "and" and "or" searches at the same time
     // (it's either all "and"s or all "or"s)
     //
     // so, if we are doing an "and" search
     // on "Any name" or "Any number" we don't want this:
     // (displayname,c,seth) && (firstname,c,seth) && (lastname,c,seth)
     // instead, just use the first term (displayname,c,seth)
     // max_attrs = 1;
     //
     // But, if we are doing an "any" search, we do want:
     // (displayname,c,seth) || (firstname,c,seth) || (lastname,c,seth)
     // max_attrs = attrs.length

so how do we fix this?

looking at RFC1558, it looks like a LDAP server could handle a mixed and / or
search filter.

and, for local mork addressbooks, we could make it work by doing an "or" search
first, and then internal to the ab code do an "and" search over those results.
  
(that should work for nsAbOutlookDirectory, as well)

but in addition to that, we'd have to make sure that nsAbBoolExprToLDAPFilter,
nsAbQueryStringToExpression and nsAbBooleanExpression do the right thing.

for example:  for ldap, that would mean turning the mixed and/or into the proper
LDAP search filter.

so this is a non-trivial bug.

note:  right now, when you do an "and" search with "any name" (or "any number")
we just search "display name" (or "home phone").

this would be too big and too risky to do during 1.3 final, so I suggest 1.4
alpha, if we think it is worth it.

here's something we could do until we have mixed and/or support.

if the user does an "and" search ("match all") we change labels in the menulist
to reflect the actual search.

("any name" -> "display name" and "any number" -> "home phone").

at least this way the menulist matches the results.  
(this approach has some issues.  for example "home phone" would appear twice.)

once we added and/or support, we could remove the code that modifies the
listitems in the menulist.

off topic, here are some spin off issues I thought of when looking through the code:

1)  "any name" should include screen name and nick name.  (for quick search and
ab advanced search)
2)  for advanced search, first, last, display name should appear individually in
the menulist, so the user can use them.

cc dmose, as he's familiar with the AB / LDAP search code.

Comment 8

[shliang]

Attachment: patch

for the "match all" search, change "any name" to "display name" and remove "any
number" since "home phone" is already there.

Comment 9

[Cavin Song]

Comment on attachment 114264 [details] [diff] [review]
patch

r=cavin.

Comment 10

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

Comment on attachment 114264 [details] [diff] [review]
patch

I think a better fix for this is to have two validity tables, 
one for local AB on for all searches & one for any.
(the current one will be used for any)

and switch between them by switching the search scope

we have LocalAB, but it will need to be updated to have
display name, any name, any number (and the rest)

then, create a new validity table, called LocalABAllSearch

this is the table for "all" searches.

when the user switches the boolean (all vs. any, lxr for
booleanChanged(event)), switch the search scope (see setSearchScope())

you'll still need to fix ABSearchDialog.js to properly land display name, any
number and any name searches.

and your change to search-attributes.properties is still needed.

Comment 11

[shliang]

Attachment: patch

Comment 12

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

Comment on attachment 118960 [details] [diff] [review]
patch

r/sr=sspitzer, with a few suggestions and questions.

1)

the "and" table should be the same as the "or" table, minus any name and any
number.

since in ::SetUpABTable(), isLocal isn't used, how about fixing the callers to
SetUpABTable(),
and doing this:

nsresult 
nsMsgSearchValidityManager::SetUpABTable(nsIMsgSearchValidityTable *aTable,
PRBool isOrTable)
{
  nsresult rv = aTable->SetDefaultAttrib(isOrTable : nsMsgSearchAttrib::Name ?
nsMsgSearchAttrib::DisplayName);
  NS_ENSURE_SUCCESS(rv,rv);

  if (isOrTable) {
    rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Name);
    NS_ENSURE_SUCCESS(rv,rv);
  }

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::DisplayName);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Email);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::AdditionalEmail);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::ScreenName);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Street);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::City);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Title);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Organization);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Department);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Nickname);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::WorkPhone);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::HomePhone);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Fax);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Pager);
  NS_ENSURE_SUCCESS(rv,rv);

  rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::Mobile);
  NS_ENSURE_SUCCESS(rv,rv);

  if (isOrTable) {
    rv = EnableDirectoryAttribute(aTable, nsMsgSearchAttrib::PhoneNumber);
    NS_ENSURE_SUCCESS(rv,rv);
  }
  return rv;
}

2)

with your fix, what happens if I'm doing an "any" search, on "any number" or
"any name", and I switch to an "and" search?

3)

how does this fix affect mail search?  do the changes to booleanChanged() and
to mailWidgets.xml cause any regressions?

Comment 13

[shliang]

resolving

Comment 14

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

> do the changes to booleanChanged() and
> to mailWidgets.xml cause any regressions?

caused a regression, see bug #202584

Comment 15

[Ninoschka Baca]

Trunk build 2003-04-22: WinXP, Mac 10.1.5, Linux RH 8

The following cases are fixed for AB Quick Search and AB Advanced Search: 
- First Name 
- Last Name
- Display Name

** The following fail during an AB Quick Search and an AB Advanced Search:
- Nick Name
- Screen Name 

Reopening due to Nick Name and Screen Name issue.

Comment 16

[shliang]

Attachment: patch

didn't realize "any name" was supposed to include nickname and screenname
(assume this is what you mean), this patch will add those

Comment 17

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

Comment on attachment 121477 [details] [diff] [review]
patch

r/sr=sspitzer

a=sspitzer, for 1.4b, since this is so trivial.

Comment 18

[shliang]

checked in additional patch

Comment 19

[Ninoschka Baca]

Trunk build 2003-04-25: WinXP, Mac 10.1.5, Linux RH 8
Verified Fixed. It looks great, thanks!