1081873 - [Loop][Contacts API] mozContacts API should trigger DOMRequest.onerror if no permissions are granted to the consumer

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Contacts, defect)

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c782f875a57e

Comment 3

[Maria Angeles Oteo (:oteo)]

[Blocking Requested - why for this release]: 
We need to resolve and uplift this patch to 2.0 because it's mandatory to resolve Bug 1045598 ([Loop] Not sharing Contact List, Loop should use identities instead of trying to use non available contact information) for Loop mobile application.

As you can see in the description of bug 1045598, not resolving this will make users, that are not allowing to share their contact list, to show the contact names in the Loop Call log which ignores completely with the permission set by the user.

Comment 4

[Andrew Overholt [:overholt]]

Not respecting user choice around their data = blocker.  I've marked bug 1045598 as well based on comment 3.

Comment 5

[Gregor Wagner [:gwagner]]

Comment on attachment 8505383 [details] [diff] [review]
v1

Review of attachment 8505383 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Thanks Gregor.

https://hg.mozilla.org/integration/b2g-inbound/rev/f461eb1653f1

Comment 7

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=648646&repo=b2g-inbound

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

It seems that the reason of the test failure is that Specialpowers.addPermission is not sync and it does not provide a way to know when the permission is properly set and so we were running the tests before actually setting the permissions properly. What I've done is to wait until we are sure that the permissions are properly set before running the tests.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=cec6eb004319

https://hg.mozilla.org/integration/b2g-inbound/rev/d917fc3dd79e

Since this issue might affect other tests I'll take a look to the SpecialPowers implementation to see if it is possible to make addPermission really sync or at least to make it notify when the permission is actually set.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d917fc3dd79e

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8505383 [details] [diff] [review]
v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Contacts API
User impact if declined: Apps won't be able to know if the user accepted or rejected a permission request to use the Contacts API and so they might behave in unexpected ways like blocking the UI until a response from the Contacts API that never comes is obtained.
Testing completed: Manual tests and unit tests added.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch: None

Comment 11

[bhavana bajaj [:bajaj]]

Waiting on javier's confirmation on verification requested in https://bugzilla.mozilla.org/show_bug.cgi?id=1045598#c14, before uplifting this.

Comment 12

[mbarone]

Hi bhavana, I've tested the bug 1045598 and it works fine.

Comment 13

[Maria Angeles Oteo (:oteo)]

Bhavana, can you give the approval for this patch? Sorry for bothering you but we are in hurry as we need to provide the application to the OEM next week and we want to verify that all is fine in 2.0 too.
Thanks a lot!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/fdf54ff20e76

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8e09a65d1cdb

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/8e09a65d1cdb

Comment 17

[(no longer active)]

FWIW, I'm removing access to navigator.mozContacts to contexts with insufficient privileges in bug 1043562.  Can I simply revert the fix for this bug when bug 1043562 lands?

Comment 18

[Reuben Morais [:reuben]]

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!, PTO 11/3-11/21) from comment #17)
> FWIW, I'm removing access to navigator.mozContacts to contexts with
> insufficient privileges in bug 1043562.  Can I simply revert the fix for
> this bug when bug 1043562 lands?

This is about privileged contexts (have access to the API, but the user is prompted for permission) and the API not providing any way for a consumer to tell that the user denied the permission request. So no, I don't think bug 1043562 has anything to do with this one.

Comment 19

[Fernando Jiménez Moreno [:ferjm]]

What Reuben said :)