[API][Contacts] navigator.mozContacts.find is returning all contacts

RESOLVED WORKSFORME

Status

()

Core
DOM: Contacts
--
major
RESOLVED WORKSFORME
5 years ago
3 years ago

People

(Reporter: zalun, Assigned: julienw)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
Running this code https://github.com/zalun/KitchenSink/blob/master/kitchensink-app/js/apis/contacts.js#L46-L60 shows 2 instead of 1 contact

First I'm cleaning everything which has "kitchensink" in note.
Then I'm adding 2 contacts (named "Tom" and "Jerry") one after another.
And Finally I'm searching for the contact named "Tom".
I'm finding both of them (as I checked).

I hope it is me doing some crazy stuff, but I looked to this from many sides. Found when writing the Cordova shim.
(Reporter)

Updated

5 years ago
Severity: normal → major
(Assignee)

Comment 1

5 years ago
Piotr, which Firefox OS version are you using?
Flags: needinfo?(zaloon)
(Reporter)

Comment 2

5 years ago
Right. It was 1.1
I've got other issues with 1.2/1.3, but this bug might be closed
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: needinfo?(zaloon)
Resolution: --- → INVALID
(Assignee)

Comment 3

5 years ago
We souldn't search by "name" in 1.1, only by givenName or lastName. But in theory this shouldn't have returned all contacts, this should have failed with an error...

I'm reopening, I'd like to try this more.

Piotr, it would help if you had an app that I could just install to try this.
Status: RESOLVED → REOPENED
Flags: needinfo?(zaloon)
Resolution: INVALID → ---
(Reporter)

Comment 4

5 years ago
This is an app (KitchenSink) 
The version which is not official one yet.
https://github.com/zalun/KitchenSink 
Just install it via App Manager please or create a zippy.
Flags: needinfo?(zaloon)
(Reporter)

Comment 5

5 years ago
I modified the test so it now contains the find example from https://developer.mozilla.org/en-US/docs/Web/API/ContactManager.find
Right now Contacts API tests in KitchenSink are failing on 1.2 and 1.3 with:
"Contacts API.find example from documentation: Something goes wrong!"
"Contacts API.create and remove a contact: find failed in cleanUp"
(Assignee)

Comment 6

5 years ago
assigning so that I don't forget, as I probably won't have the time today.
Assignee: nobody → felash
I tried Piotr's code, and also added some "logging". Seeing as the request can return a DOMError in its error field, I tried logging that one out:

callback(false, test, 'Something goes wrong! err' + request.error.name + " BOOM");

Unfortunately I only got an "UnknownError" name, which wasn't very helpful in trying to trace the source of the error...
(Assignee)

Comment 8

5 years ago
Yeah, we could use more explicit errors.

I'm quite sure we should actually get an error in this case in Firefox 1.1, but it should work in Firefox 1.2. The thing that should never happen is returning all contacts.
I used the Kitchen Sink app[1] to test this together with the App Manager / Simulator.
- 1.1 error is present
- 1.2 error is present
- 1.3 no error

Is that expected?

[1] https://github.com/zalun/KitchenSink/
(Reporter)

Comment 10

5 years ago
1.3 - the error breaks KitchenSink for me - I will investigate it tomorrow.
(In reply to Fred Wenzel [:wenzel] from comment #9)
> - 1.3 no error

Yeah I think I misinterpreted the output.
(Assignee)

Comment 12

5 years ago
(In reply to Fred Wenzel [:wenzel] from comment #9)
> I used the Kitchen Sink app[1] to test this together with the App Manager /
> Simulator.
> - 1.1 error is present
> - 1.2 error is present
> - 1.3 no error
> 
> Is that expected?

Yep sorry, 1.2 does not have the name index. (for reference, this was bug 898337).
This is exactly what's expected.

(still found no time to look at this, sorry :( )
(Reporter)

Comment 13

5 years ago
1.3 was throwing because new mozContacts().init is no longer a function. After changing it - the situation is the same as it was on 1.2

I removed 'name' from the filterBy as suggested. No difference.
The test looks like that at the moment: https://github.com/zalun/KitchenSink/blob/c74018968a9caf00a6c2012b5bab395336a0e43e/kitchensink-app/js/apis/contacts.js#L82-L103

And the response: "ERROR: Contacts API.find example from documentation: Something goes wrong!"

The other find which is failing is in cleanUp function where it searches by givenName only.
I also checked with the filterOp: 'startsWith', but no difference as well
(Assignee)

Comment 14

5 years ago
So, several things are wrong in your code:

* firstName is not an index, you want "givenName" [1] [2]
* nickname is not an index [1]
* filterValue needs to be an array [3]

[1] https://github.com/zalun/KitchenSink/blob/master/kitchensink-app/js/apis/contacts.js#L88
[2] https://github.com/zalun/KitchenSink/blob/master/kitchensink-app/js/apis/contacts.js#L122
[3] https://github.com/zalun/KitchenSink/blob/master/kitchensink-app/js/apis/contacts.js#L25

I'll file a bug to provide better errors, because currently it's really guessing which indices are available...
(Assignee)

Comment 15

5 years ago
Also Piotr, I'd really want to know if your comment 0 was accurate: did you get all contacts? Can you share a code that would still do this in 1.1 (and maybe on other versions too?)
(Reporter)

Comment 16

5 years ago
Thanks. 
Tests fixed - https://github.com/zalun/KitchenSink/commit/c543aab3d097cb6be578c80fcf476fc53a130199
This is working fine in 1.3 (**+)
1.2 reports an issue (*F+) "Contacts API.create, search and remove a contact: find test failed. Expected: 1, Found: 0"
(Reporter)

Comment 17

5 years ago
in 1.1 This is providing 2 results (expecting one)
https://github.com/zalun/KitchenSink/blob/4c5b45c498225696c176545b4f5f99249847b011/kitchensink-app/js/apis/contacts.js#L41-L61

Be careful as cleanUp is deleting everything (after running KitchenSink there is only Tom contact left, Jerry is deleted inside test)

CleanUp:
https://github.com/zalun/KitchenSink/blob/4c5b45c498225696c176545b4f5f99249847b011/kitchensink-app/js/apis/contacts.js#L14-L39
(Assignee)

Comment 18

5 years ago
(In reply to Piotr Zalewa [:zalun] from comment #16)
> Thanks. 
> Tests fixed -
> https://github.com/zalun/KitchenSink/commit/
> c543aab3d097cb6be578c80fcf476fc53a130199
> This is working fine in 1.3 (**+)
> 1.2 reports an issue (*F+) "Contacts API.create, search and remove a
> contact: find test failed. Expected: 1, Found: 0"

I think that in 1.2 and 1.1 you need to use the old "init" method, and that would be the reason of the failure for 1.2 at least.
I'm not sure how you can feature-test this... Maybe "if (mozContact.prototype.init)" ?

Reuben, how can we make a code that works with both the old and the new mozContact API ?
Flags: needinfo?(reuben.bmo)
(Reporter)

Comment 19

5 years ago
Good idea - I will work on Contacts API for Cordova so it will work with both versions
(In reply to Julien Wajsberg [:julienw] from comment #18)
> I think that in 1.2 and 1.1 you need to use the old "init" method, and that
> would be the reason of the failure for 1.2 at least.
> I'm not sure how you can feature-test this... Maybe "if
> (mozContact.prototype.init)" ?
> 
> Reuben, how can we make a code that works with both the old and the new
> mozContact API ?

It seems we should have a dummy init() method on the new WebIDL mozContact object.  We broke backward compatibility by removing it completely.
(In reply to Julien Wajsberg [:julienw] from comment #18)
> I think that in 1.2 and 1.1 you need to use the old "init" method, and that
> would be the reason of the failure for 1.2 at least.
> I'm not sure how you can feature-test this... Maybe "if
> (mozContact.prototype.init)" ?
> 
> Reuben, how can we make a code that works with both the old and the new
> mozContact API ?

Due to the way the mozContact ctor was registered before WebIDL, you have to do something like:

  if ("init" in (new mozContact())) {

Alternatively, you can use that fact to do an indirect check that doesn't create an object:

  if (window.mozContact.prototype) {

Which of course doesn't work if someone sets the prototype.
Flags: needinfo?(reuben.bmo)
(Assignee)

Comment 22

5 years ago
Reuben, would something like this work:

  var data = {
    givenName: "Mark",
    familyName: "Smith
  };

  var contact = new mozContact(data);
  if ("init" in contact) {
    contact.init(data);
  }

?
Flags: needinfo?(reuben.bmo)
(In reply to Julien Wajsberg [:julienw] from comment #22)
>     givenName: "Mark",
>     familyName: "Smith

Surely you mean ["Mark"] and ["Smith"] ;)

>   var contact = new mozContact(data);
>   if ("init" in contact) {
>     contact.init(data);
>   }

Yes, that would work.
Flags: needinfo?(reuben.bmo)
(Assignee)

Comment 24

5 years ago
Yeah, I forgot that givenName and familyName are arrays too ;)
(Reporter)

Comment 25

5 years ago
Cool - this code is working fine in 1.2 and 1.3
https://github.com/zalun/KitchenSink/blob/c129f09b6b7458f53a918dbbae33c52f310dc9f1/kitchensink-app/js/apis/contacts.js#L89-L96

I'm now investigating search issues - is there a place where I can find on which fields one may and on which may not search?

Thanks
(Assignee)

Comment 26

5 years ago
The best place right now is directly at the Source:

https://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/ContactDB.jsm#166

The "tel" index is special cased.
The "LowerCase" indices are used for the "startsWith" search.

The same location for v1.1 is: https://mxr.mozilla.org/mozilla-b2g18/source/dom/contacts/fallback/ContactDB.jsm#168

v1.2 is currently https://mxr.mozilla.org/mozilla-beta/source/dom/contacts/fallback/ContactDB.jsm#165
(Assignee)

Comment 27

5 years ago
I updated MDN with what we said here:

https://developer.mozilla.org/en-US/docs/Web/API/mozContact
https://developer.mozilla.org/en-US/docs/WebAPI/Contacts

Hopefully we'll find soon why this doesn't work correctly in v1.1 :)
(Assignee)

Comment 28

3 years ago
I think this works now.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.