Closed Bug 804650 Opened 7 years ago Closed 7 years ago

[contacts] API sorting problems

Categories

(Core :: DOM: Device Interfaces, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: arcturus, Assigned: gwagner)

Details

Attachments

(2 files)

The current contacts API allow us to sort by 'given' or 'family' name.

If we ask for one of the sorting types, let's say 'family' and the contacts that we have just have 'given' the sorting returned makes the ordering incorrect.

Could be useful to be able to sort by two fields, something like:

SELECT * FROM Contacts ORDER BY given, family

(I know, indexeddb is not sql, but at least hope to explain the ideal solution better)
blocking-basecamp: --- → ?
Nice to have but not in this version
blocking-basecamp: ? → -
Hi!

IMHO, a bad ordering in the list of contacts is a bit more than a nice to have.

Cheers!
Component: Gaia → DOM: Device Interfaces
Product: Boot2Gecko → Core
This is a gecko bug. Lets discuss this in the gecko triage as well.
blocking-basecamp: - → ?
Gregor, can you provide a bit more justification for why you think this should block?
Flags: needinfo?(anygregor)
Nevermind, we discussed it more during triage and think it should block :)
Assignee: nobody → anygregor
blocking-basecamp: ? → +
Flags: needinfo?(anygregor)
Casey, do we have any specifications on contacts sorting?
Attached patch patchSplinter Review
Attachment #675782 - Flags: review?(shu)
Comment on attachment 675782 [details] [diff] [review]
patch

High level comments followed by nitpicks. Looks good to me overall.

A potential problem is that |sortfunction| will never return 0. For two records that are completely the same it will always return 1 or -1. Since I think |Array.sort| is stable, this could change the relative ordering of of otherwise-equal contacts pre- and post-sorting. For example,

  var contacts = [{ properties: { givenName: ['Gregor'] }, id: 0 },
                  { properties: { givenName: ['Gregor'] }, id: 1 }];

might get sorted to:

  var contacts = [{ properties: { givenName: ['Gregor'] }, id: 1 },
                  { properties: { givenName: ['Gregor'] }, id: 0 }];

Whether this is an actual problem or not, I don't know. Just pointing it out.

> +      let sortBy = findOptions.sortBy === "familyName" ? [ "familyName", "givenName"] : [ "givenName" , "familyName"];

Nit: put a space after closing the array.

r+ with nitpicks
Attachment #675782 - Flags: review?(shu) → review+
(In reply to Shu-yu Guo [:shu] from comment #8)
> Comment on attachment 675782 [details] [diff] [review]
> patch
> 
> High level comments followed by nitpicks. Looks good to me overall.
> 
> A potential problem is that |sortfunction| will never return 0. For two
> records that are completely the same it will always return 1 or -1. Since I
> think |Array.sort| is stable, this could change the relative ordering of of
> otherwise-equal contacts pre- and post-sorting. For example,
> 
>   var contacts = [{ properties: { givenName: ['Gregor'] }, id: 0 },
>                   { properties: { givenName: ['Gregor'] }, id: 1 }];
> 
> might get sorted to:
> 
>   var contacts = [{ properties: { givenName: ['Gregor'] }, id: 1 },
>                   { properties: { givenName: ['Gregor'] }, id: 0 }];
> 
> Whether this is an actual problem or not, I don't know. Just pointing it out.

Good point but I don't think this matters here. We don't have any mechanism in place when 2 contacts have the same given and last name.
https://hg.mozilla.org/mozilla-central/rev/3e2ec828f051
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.