Closed Bug 1081873 Opened 10 years ago Closed 10 years ago

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

Categories

(Core Graveyard :: DOM: Contacts, defect)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

(blocking-b2g:2.0+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
mozilla36
blocking-b2g 2.0+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: ferjm, Assigned: ferjm)

References

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Assignee: nobody → ferjmoreno
Component: Gaia::Loop → DOM: Device Interfaces
Product: Firefox OS → Core
Component: DOM: Device Interfaces → DOM: Contacts
Summary: [Loop][Contacts API] mozContacts.find should trigger onerror callback if no permissions are granted to the consumer → [Loop][Contacts API] mozContacts API should trigger DOMRequest.onerror if no permissions are granted to the consumer
Attached patch v1 (obsolete) — Splinter Review
[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.
blocking-b2g: --- → 2.0?
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Severity: normal → critical
Not respecting user choice around their data = blocker.  I've marked bug 1045598 as well based on comment 3.
blocking-b2g: 2.0? → 2.0+
Comment on attachment 8505383 [details] [diff] [review]
v1

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

Thanks!
Attachment #8505383 - Flags: review?(anygregor) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/d917fc3dd79e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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
Attachment #8505383 - Flags: approval-mozilla-b2g34?
Attachment #8505383 - Flags: approval-mozilla-b2g32?
Waiting on javier's confirmation on verification requested in https://bugzilla.mozilla.org/show_bug.cgi?id=1045598#c14, before uplifting this.
Hi bhavana, I've tested the bug 1045598 and it works fine.
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!
Flags: needinfo?(bbajaj)
Flags: needinfo?(bbajaj)
Attachment #8505383 - Flags: approval-mozilla-b2g34?
Attachment #8505383 - Flags: approval-mozilla-b2g34+
Attachment #8505383 - Flags: approval-mozilla-b2g32?
Attachment #8505383 - Flags: approval-mozilla-b2g32+
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?
Flags: needinfo?(ferjmoreno)
(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.
What Reuben said :)
Flags: needinfo?(ferjmoreno)
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.