Closed
Bug 883269
Opened 12 years ago
Closed 7 years ago
[contacts] characters with non-ascii characters (like umlauts) are sorted always into first positions for letter
Categories
(Core Graveyard :: DOM: Contacts, defect, P2)
Tracking
(blocking-b2g:-)
RESOLVED
WONTFIX
blocking-b2g | - |
People
(Reporter: aryx, Unassigned)
References
Details
Attachments
(1 file)
Unagi with Gaia 1.1.0.0-prerelease 20130614070213
Contacts with non-ASCII character like umlauts (ä, ö, ü), accents (á) etc. are always sorted in front of the other contacts with the same leading letter of the contact name and only containing ascii characters.
Example:
Actual sort order:
Jörg Amigo
Julian Schönwetter
Jana Prima
Expected sort order:
Jana Prima
Jörg Amigo
Julian Schönwetter
Comment 1•12 years ago
|
||
What's the language on your device?
The sorting depends on your language selection. If you have selected a language where the 'umlauts' are not part of the language you will get the strange sorting behavior.
Comment 2•12 years ago
|
||
I just tried it with german and I get your expected sort order.
Reporter | ||
Comment 3•12 years ago
|
||
The language is set to German. Even after switching to English, closing the Contacts app, launching it, closing it, switching back to German and opening the Contacts, the issue persists.
Comment 4•12 years ago
|
||
The sort order is cached. We have to clear the sorting-cache if we change the language.
Updated•11 years ago
|
Assignee: nobody → mihai
Comment 5•11 years ago
|
||
Add contacts to the group list in order and not just append them.
Attachment #770735 -
Flags: review?(francisco.jordano)
Updated•11 years ago
|
blocking-b2g: leo? → leo+
Comment 6•11 years ago
|
||
Comment on attachment 770735 [details]
Pull Request #10764 - Sort contacts in list
Tried on the phone, working fine.
Thanks Mihai, could you squash your commits and land this?
Attachment #770735 -
Flags: review?(francisco.jordano) → review+
Comment 7•11 years ago
|
||
Thanks Francisco! Landed on master:
https://github.com/mozilla-b2g/gaia/commit/b068bef2bfbb759271d4446afa5d4b2207c44ae9
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 8•11 years ago
|
||
I'm sorry I didn't see this bug until now, but I have some concerns about the final solution:
1) We're sorting twice on the device; once in gecko and once in gaia. We now ignore the gecko sorting.
2) This is going to add a lot of CPU usage when loading a large list. The addToGroup() function does a very simple bubble sort which is effectively O(n^2). This is ok for adding one contact here and there, but is probably inadequate for loading a large list. This change now uses this sort for the bulk loading of contacts at start.
Mihai, have you tested this with the reference workloads? Try "make reference-workload-heavy" to install 1000 contacts on the phone. You may need to add debug to see how long the load takes.
Out of curiousity, is there a reason we did not pursue fixing the sort caching issue in gecko mentioned in comment 4?
Flags: needinfo?(mihai)
Comment 9•11 years ago
|
||
I just tested master with the reference-workload-heavy data set. With this change scrolling while data is loading is now janky.
I'm very sorry, but I really think this should be backed out and the sort caching issue should be fixed in gecko.
Comment 10•11 years ago
|
||
After chatting with Ben on IRC, we agreed that it's best if this patch is reverted due to performance concerns and the gecko approach is pursued first.
Thanks for checking all this, Ben!
Flags: needinfo?(mihai)
Comment 11•11 years ago
|
||
Commit reverted:
https://github.com/mozilla-b2g/gaia/commit/6d79318e980613b65838dfb990a2cd75a9c0e1ff
Thanks for understanding my concerns Mihai!
Gregor, Reuben, what are your thoughts on fixing the sort cache for language change issue? I can take a look on Monday if you don't have the bandwidth.
Status: RESOLVED → REOPENED
Flags: needinfo?(reuben.bmo)
Flags: needinfo?(anygregor)
Resolution: FIXED → ---
Comment 12•11 years ago
|
||
Huh, wth. This bug fell of the cracks, I'm pretty sure I had a patch for it before I lost my data…
Assignee: mihai → reuben.bmo
Component: Gaia::Contacts → DOM: Device Interfaces
Flags: needinfo?(reuben.bmo)
Flags: needinfo?(anygregor)
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Updated•11 years ago
|
Priority: -- → P1
Whiteboard: [c= ]
Comment 13•11 years ago
|
||
So I have a patch to invalidate our sorting cache when the language changes, but it doesn't help, because we're using String.prototype.localeCompare in ContactsDB and the locale in the parent is always set to en_us/C. (Yay!)
Comment 14•11 years ago
|
||
No longer blocking for 1.1, we agreed in triage that this is only a major issue if the contact starts with a non-ascii character, which will be uncommon.
blocking-b2g: leo+ → -
Comment 16•11 years ago
|
||
Is there already a bug to support the 2 additional arguments to localeCompare (see MDN [1]) ?
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare
Comment 17•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #16)
> Is there already a bug to support the 2 additional arguments to
> localeCompare (see MDN [1]) ?
>
> [1]
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/String/localeCompare
That would be Bug 866301.
Updated•11 years ago
|
Component: DOM: Device Interfaces → DOM: Contacts
Comment 18•11 years ago
|
||
Clearing the perf keyword since this is currently a functional problem. The perf keyword was only added because the attempted solution created a perf problem. Any final solution should be cognizant of performance impact (like all changes), but this is not directly a performance issue.
Keywords: perf
Whiteboard: [c= ]
Comment 19•9 years ago
|
||
Looks like we have localeCompare with the additional arguments now. This bug could be actionable now ?
Updated•7 years ago
|
Severity: normal → major
Priority: P1 → P2
Comment 20•7 years ago
|
||
Contacts API is gone.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 7 years ago
Resolution: --- → WONTFIX
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•