1129844 - UserDictionary word list should sort alphabetically

Status: RESOLVED FIXED

Description

[John Lu [:mnjul][Please NI or I'll miss]]

I consulted :omega and the list should be sorted alphabetically, so the implementation at bug 1102831 is not ideal yet.

Comment 1

[GitHub Autolander Bot]

Created attachment 8561924 [details] [review]
[PullReq] mnjul:bug_1129844_user_dict_lexical_sort to mozilla-b2g:master

Comment 2

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8561924 [details] [review]
[PullReq] mnjul:bug_1129844_user_dict_lexical_sort to mozilla-b2g:master

Rationale behind the patch is that I don't want both the view and the model to do the sorting, and in this patch only the model is doing the sorting. As such, model returns the full, sorted list of words on modification (add, remove, replace, although it isn't used in removal and in replacement where the new word is some other old word). View needs to have another word-to-wordElem mapping, in order to rearrange the wordElem from the word list returned from model for each modification.

UDListView._appendList is replaced by UDListView._rearrangeList. This new function re-order wordElems, and generate new wordElem for words that haven't bee displayed (such as initialization, add, and replace).

Tim, please check if the patch looks good and I'll proceed to write unit tests.

Side note: we can't enjoy localeCompare's full power due to bug 866301. i.e., something like a.localeCompare(b, navigator.mozL10n.language.code, {sensitivity: 'base'}) doesn't work on a real device. The sorting code is manually written to sort case-insensitive, but disregards diacritics, as opposed to what localCompare would've done.

Comment 3

[John Lu [:mnjul][Please NI or I'll miss]]

...I always forget to type one sentence.

(In reply to John Lu [:mnjul] [MoCoTPE] from comment #2)
> Comment on attachment 8561924 [details] [review]
> [PullReq] mnjul:bug_1129844_user_dict_lexical_sort to mozilla-b2g:master
> Side note: we can't enjoy localeCompare's full power due to bug 866301.
> i.e., something like a.localeCompare(b, navigator.mozL10n.language.code,
> {sensitivity: 'base'}) doesn't work on a real device. The sorting code is
> manually written to sort case-insensitive, but disregards diacritics, as
> opposed to what localCompare would've done.

That is, true lexicographical sort is not yet possible.

Comment 4

[John Lu [:mnjul][Please NI or I'll miss]]

Comment on attachment 8561924 [details] [review]
[PullReq] mnjul:bug_1129844_user_dict_lexical_sort to mozilla-b2g:master

Gaia Try hook isn't up for master so I'm requesting review first.

Tests have been amended.

As we've probably had guessed, removing that <a> messes up with style. So DOM structure is retained and I use pointer-events: none on <a>, such that click target can be <li> correctly.

Comment 5

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/ff992089016388feb568e4c3b8a1685f7056faff