Closed Bug 883269 Opened 11 years ago Closed 6 years ago

[contacts] characters with non-ascii characters (like umlauts) are sorted always into first positions for letter


(Core Graveyard :: DOM: Contacts, defect, P2)

Gonk (Firefox OS)



blocking-b2g -


(Reporter: aryx, Unassigned)




(1 file)

Unagi with Gaia 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.


Actual sort order:
Jörg Amigo
Julian Schönwetter
Jana Prima

Expected sort order:
Jana Prima
Jörg Amigo
Julian Schönwetter
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.
I just tried it with german and I get your expected sort order.
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.
The sort order is cached. We have to clear the sorting-cache if we change the language.
Assignee: nobody → mihai
blocking-b2g: --- → leo?
Depends on: 881904
Add contacts to the group list in order and not just append them.
Attachment #770735 - Flags: review?(francisco.jordano)
blocking-b2g: leo? → leo+
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+
Thanks Francisco! Landed on master:
Closed: 11 years ago
Resolution: --- → FIXED
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)
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.
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)
Commit reverted:

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.
Flags: needinfo?(reuben.bmo)
Flags: needinfo?(anygregor)
Resolution: FIXED → ---
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
Priority: -- → P1
Whiteboard: [c= ]
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!)
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+ → -
Depends on: 866301
Unassigning until this is actionable again.
Assignee: reuben.bmo → nobody
Is there already a bug to support the 2 additional arguments to localeCompare (see MDN [1]) ?

(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]
> Global_Objects/String/localeCompare

That would be Bug 866301.
Component: DOM: Device Interfaces → DOM: Contacts
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= ]
Looks like we have localeCompare with the additional arguments now. This bug could be actionable now ?
Severity: normal → major
Priority: P1 → P2
Contacts API is gone.
Closed: 11 years ago6 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.