UserDictionary word list should sort alphabetically

RESOLVED FIXED in 2.2 S6 (20feb)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mnjul, Assigned: mnjul)

Tracking

unspecified
2.2 S6 (20feb)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

I consulted :omega and the list should be sorted alphabetically, so the implementation at bug 1102831 is not ideal yet.
Created attachment 8561924 [details] [review]
[PullReq] mnjul:bug_1129844_user_dict_lexical_sort to mozilla-b2g:master
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.
Attachment #8561924 - Flags: feedback?(timdream)
Assignee: nobody → jlu
Status: NEW → ASSIGNED
Whiteboard: [p=1]
Target Milestone: --- → 2.2 S7 (6mar)
...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.
Attachment #8561924 - Flags: feedback?(timdream) → feedback+
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.
Attachment #8561924 - Flags: review?(timdream)
Attachment #8561924 - Flags: review?(timdream) → review+
Keywords: checkin-needed
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: 2.2 S7 (6mar) → 2.2 S6 (20feb)
You need to log in before you can comment on or make changes to this bug.