Closed
Bug 1129844
Opened 10 years ago
Closed 10 years ago
UserDictionary word list should sort alphabetically
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.2 S6 (20feb)
People
(Reporter: mnjul, Assigned: mnjul)
References
Details
(Whiteboard: [p=1])
Attachments
(1 file)
I consulted :omega and the list should be sorted alphabetically, so the implementation at bug 1102831 is not ideal yet.
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → jlu
Status: NEW → ASSIGNED
Whiteboard: [p=1]
Target Milestone: --- → 2.2 S7 (6mar)
Assignee | ||
Comment 3•10 years ago
|
||
...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.
Updated•10 years ago
|
Attachment #8561924 -
Flags: feedback?(timdream) → feedback+
Assignee | ||
Comment 4•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8561924 -
Flags: review?(timdream) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 5•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/ff992089016388feb568e4c3b8a1685f7056faff
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
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.
Description
•