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

RESOLVED FIXED in Firefox 36, Firefox OS v2.0

Status

()

Core
DOM: Contacts
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ferjm, Assigned: ferjm)

Tracking

unspecified
mozilla36
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

4 years ago
Assignee: nobody → ferjmoreno
(Assignee)

Updated

4 years ago
Component: Gaia::Loop → DOM: Device Interfaces
Product: Firefox OS → Core
Component: DOM: Device Interfaces → DOM: Contacts
(Assignee)

Updated

4 years ago
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
[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
Blocks: 1062883
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+
sorry had to back this out for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=648646&repo=b2g-inbound
(Assignee)

Comment 8

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(Assignee)

Comment 10

4 years ago
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.

Comment 12

4 years ago
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)

Updated

4 years ago
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+
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/fdf54ff20e76
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
status-firefox34: --- → wontfix
status-firefox35: --- → wontfix
status-firefox36: --- → fixed
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/8e09a65d1cdb
status-b2g-v2.0M: --- → fixed
Flags: in-testsuite+

Comment 17

4 years ago
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.
(Assignee)

Comment 19

4 years ago
What Reuben said :)
Flags: needinfo?(ferjmoreno)
You need to log in before you can comment on or make changes to this bug.