Comment 90 for bug 956618

Comment on attachment 8557496
Patch v2

Review of attachment 8557496:
-----------------------------------------------------------------

Thanks a lot Suyash for your quick response to my request to pick this up - I am really getting tired of referencing and explaining this bug (and workaround) in other bugs where users have been facing problems due to this major flaw which practically disables the entire nickname functionality for a range of scenarios.

I'll set feedback+ because we're on the right track here; but the devil - as always - in the detail (i.e. this patch as-is won't work yet as expected)...

::: mailnews/addrbook/src/nsAbAutoCompleteSearch.js
@@ +141,5 @@
> + // is the nick name for the card or at least in the beginning of it.
> + let nick = aCard.getProperty("NickName", "").toLocaleLowerCase();
> + aSearchString = aSearchString.toLocaleLowerCase();
> + if (nick.indexOf(aSearchString) == 0)
> + return BEST + 1;

(In reply to Magnus Melin from comment #86)
> You don't need the nick == aSearchString
> nick should act as a "super-best" match. so the score should be 1 better
> than if it matched in display name - as is they would be equal (=> we need BEST + 1)

True, but still not sufficient for ensuring absolute top ranking for *full* nickname matches over *partial* nickname matches, e.g.:
Card1: Nickname="foo" (nick==searchphrase; nickIndex==0)
Card2: Nickname="foobar" (nick!=searchphrase; nickIndex==0)
Searchphrase: "foo"

Autocomplete result (wrong):
Card2 (partial nick match)
Card1 (full nick match)

To make the nickname design work reliably, full nickname match must obviously supersede partial nickname match, so we would need to adjust the scoring accordingly:

if (nick == aSearchString)
  return BEST+2;
if (nickIndex == 0)
  return BEST+1;

However, I'm not convinced yet if making initial nickname matches rank higher than anything else will really work out well, e.g.:

Card1: Display Name="Tom" popularityIndex=10
Card2: Nickname="TomFoo" popularityIndex=5
Card3: Nickname="TomBar" popularityIndex=4
Searchphrase: "Tom"

Autocomplete result:
Card2 (partial nickname match, pi=5)
Card3 (partial nickname match, pi=4)
Card1 (full display name match, pi=10)

So I'm not sure if all results with partial matches on nickname should rank higher than full match on display name; should they? (I think not). Worse, PLEASE let's be aware that any forced top scorings that are not based on frecency will further undermine even the current rudimentary frequency implemtation of "popularityIndex":

Card1: Display Name="Tom" popularityIndex=1000
Card2: Nickname="TomFoo" popularityIndex=5
Card3: Nickname="TomBar" popularityIndex=4
Card4: Nickname="Tim" popularityIndex=6
Card5: Nickname="TD" popularityIndex=7
Searchphrase: "T"

Autocomplete result:
(all partial nickname matches top-listed, ordered by popularity:)
Card5
Card4
Card2
Card3
(then, potentially far down the list:)
Card1 (initial Display name match; popularityIndex=1000(!))

So in this case, even for partial nickname matches from single-character(!) search phrase, ALL partial nickname matches will be toplisted (although they are not popular at all, and the partial match on their nicknames is entirely insignificant!), whereas highly popular Card1 with an inital match on Display name will rank lower than all the insignificant matches.

Undermining "frecency"-based sorting more is imo a slippery approach because we'll never find a one-for-all algorithm to predict which address the user actually wants, but "frecency" (linked up with the actual user input) will certainly be one of our best bets. I think. These things are actually pretty complex...

Please correct me if I'm wrong, but I conclude that it'll be safer if we just fix the main usecase of this bug, which is FULL nickname matches; let the rest be handled by our existing algorithm. So I suspect that this should be sufficient:

if (nick == aSearchString)
  return BEST+1;

Also, since full nickname matches (per inherent design) must really always be toplisted without exception (regardless of any other sorting algorithms we might think of), I wonder if we should leave some room in the score for pushing other matches into the "best+" range:

if (nick == aSearchString)
  return BEST+100;

Further technical reviews might want to test and improve performance against large ABs.