804650 - [contacts] API sorting problems

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Contacts just with given name ordered by family name

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)

Comment 1

[David Scravaglieri [:scravag]]

Nice to have but not in this version

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Hi!

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

Cheers!

Comment 3

[Gregor Wagner [:gwagner]]

This is a gecko bug. Lets discuss this in the gecko triage as well.

Comment 4

[Andrew Overholt [:overholt]]

Gregor, can you provide a bit more justification for why you think this should block?

Comment 5

[Andrew Overholt [:overholt]]

Nevermind, we discussed it more during triage and think it should block :)

Comment 6

[Gregor Wagner [:gwagner]]

Casey, do we have any specifications on contacts sorting?

Comment 7

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 8

[Shu-yu Guo [: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

Comment 9

[Gregor Wagner [:gwagner]]

(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.

Comment 10

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e2ec828f051

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3e2ec828f051

Comment 12

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0859decaeafc