Comment 88 for bug 956618

Revision history for this message
In , Mkmelin+mozilla (mkmelin+mozilla) wrote :

Comment on attachment 8556897
Patch v1

Review of attachment 8556897:
-----------------------------------------------------------------

::: mailnews/addrbook/src/nsAbAutoCompleteSearch.js
@@ +128,5 @@
> * Gets the score of the (full) address, given the search input. We want
> * results that match the beginning of a "word" in the result to score better
> * than a result that matches only in the middle of the word.
> *
> + * @param aCard - The card whose score is being decided

lowercase t?

@@ +139,5 @@
> +
> + // We will firstly check if the search term provided by the user
> + // is the nick name for the card or at least in the beginning of it.
> + let nick = aCard.getProperty("NickName", "");
> + nick = nick.toLocaleLowerCase();

you can just put .toLocaleLowerCase() after getPropery above

@@ +140,5 @@
> + // We will firstly check if the search term provided by the user
> + // is the nick name for the card or at least in the beginning of it.
> + let nick = aCard.getProperty("NickName", "");
> + nick = nick.toLocaleLowerCase();
> + aSearchString = aSearchString.toLocaleLowerCase();

remove the duplication of this below.

@@ +143,5 @@
> + nick = nick.toLocaleLowerCase();
> + aSearchString = aSearchString.toLocaleLowerCase();
> + let nickIndex = nick.indexOf(aSearchString);
> + if (nick == aSearchString || nickIndex == 0)
> + return BEST;

You don't need the nick == aSearchString
But, this doesn't quite do what this bug requests. What is requested is that 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 (BEST + 1)

Please also adjust the test for this. There should be one test where the nick is the beginning of another persons name.

::: mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch6.js
@@ +139,5 @@
> card.setProperty("PopularityIndex", element.popularityIndex);
> card.firstName = element.firstName;
> card.lastName = element.lastName;
> + if (element.NickName)
> + card.setProperty("NickName", element.NickName);

You don't need the if I think. All the other props are also just set to undefined if not set. Also, please lowerCamelCase NickName