Enable mozContacts on Firefox desktop

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
6 years ago
2 years ago

People

(Reporter: florian, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 724913 [details] [diff] [review]
Patch

mozContacts looks like it could be useful on Firefox Desktop for our Talkilla project but it's currently disabled. After experimenting a bit and discussing with gwagner on IRC, it turns out the only reason why it's disabled seems to be that it doesn't work because PermissionPromptHelper.jsm and ContactService.jsm are never imported on desktop (and the ContactsService expects them to be imported by a parent process; which doesn't exist for Firefox desktop).

To import them only if the current process initializing mozContacts is the 'parent process', I adapted some code from dom/network/src/NetworkStatsManager.js.

Enabling this doesn't seem to cause any security/privacy issue, as web applications need to receive the contacts-{read,write,create} permissions before being able to use the API.
Attachment #724913 - Flags: review?(anygregor)
To what kind of content would that be available?
Hardware: x86 → All
(Reporter)

Comment 2

6 years ago
(In reply to Mounir Lamouri (:mounir) from comment #1)
> To what kind of content would that be available?

A web app running in the Social API sidebar that would enable easy WebRTC calls between Firefox users, and phone calls through PSTN gateways.
(In reply to Florian Quèze [:florian] [:flo] from comment #0)
> Created attachment 724913 [details] [diff] [review]
> Patch
> 
> mozContacts looks like it could be useful on Firefox Desktop for our
> Talkilla project but it's currently disabled. After experimenting a bit and
> discussing with gwagner on IRC, it turns out the only reason why it's
> disabled seems to be that it doesn't work because PermissionPromptHelper.jsm
> and ContactService.jsm are never imported on desktop (and the
> ContactsService expects them to be imported by a parent process; which
> doesn't exist for Firefox desktop).

In single-process mode, the process is considered as a parent process. What you never get is an explicit child process.

> To import them only if the current process initializing mozContacts is the
> 'parent process', I adapted some code from
> dom/network/src/NetworkStatsManager.js.


In b2g we import them in shell.js (https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#7). Doing it in ContactManager.js which is the DOM facing part doesn't look right to me.
(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> (In reply to Mounir Lamouri (:mounir) from comment #1)
> > To what kind of content would that be available?
> 
> A web app running in the Social API sidebar that would enable easy WebRTC
> calls between Firefox users, and phone calls through PSTN gateways.

What is the privilege status of that "web app"? We shouldn't have any regular content be able to use mozContacts.
(Reporter)

Comment 5

6 years ago
(In reply to Mounir Lamouri (:mounir) from comment #4)
> (In reply to Florian Quèze [:florian] [:flo] from comment #2)
> > (In reply to Mounir Lamouri (:mounir) from comment #1)
> > > To what kind of content would that be available?
> > 
> > A web app running in the Social API sidebar that would enable easy WebRTC
> > calls between Firefox users, and phone calls through PSTN gateways.
> 
> What is the privilege status of that "web app"? We shouldn't have any
> regular content be able to use mozContacts.

It will have additonnal permissions. It needs contacts, but it also needs access to the camera, which we obviously can't give to regular content without the user approving it each time.

It will be loaded from the internet, but only from whitelisted URLs (possibly even only from mozilla domains).
(Reporter)

Comment 6

6 years ago
(In reply to Fabrice Desré [:fabrice] from comment #3)
> (In reply to Florian Quèze [:florian] [:flo] from comment #0)

> > To import them only if the current process initializing mozContacts is the
> > 'parent process', I adapted some code from
> > dom/network/src/NetworkStatsManager.js.
> 
> 
> In b2g we import them in shell.js
> (https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.
> js#7). Doing it in ContactManager.js which is the DOM facing part doesn't
> look right to me.

I don't see why it's a problem. Is the situation different from what exists in NetworkStatsManager.js?
(In reply to Florian Quèze [:florian] [:flo] from comment #6)

> I don't see why it's a problem. Is the situation different from what exists
> in NetworkStatsManager.js?

I guess NetworkStatsManager.js should be fixed too. We had more than one bug where people had expectations of something working in the child when it should be parent-only or vice versa, so I would rather keep them clearly separated.
(Reporter)

Comment 8

6 years ago
(In reply to Fabrice Desré [:fabrice] from comment #7)

> We had more than one bug
> where people had expectations of something working in the child when it
> should be parent-only or vice versa, so I would rather keep them clearly
> separated.

I think I still don't fully understand your point. Is there something wrong in the way isParentProcess is computed that could make it import the 2 JS modules inside a child process?
My point is not that your code does not work, but that this should not be done this way.
(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> It will be loaded from the internet, but only from whitelisted URLs
> (possibly even only from mozilla domains).

FWIW, I think that is a terrible thing to do but as long as regular content can't access or even know the existence of mozContacts, that's fine.
Comment on attachment 724913 [details] [diff] [review]
Patch

I don't like this approach. The DOM facing part shouldn't be responsible for bringing up the ContactService.

We already had some discussion about desktop integration in bug 674720.
Attachment #724913 - Flags: review?(anygregor) → review-
We recently disabled more parts of the API in Firefox desktop because it's unsafe in a single process environment. If/when we want to fix this bug, it needs a sec review.
FYI - we currently import these for DEBUG gaia builds running inside of firefox, so the contacts API should work there.
(Reporter)

Comment 14

5 years ago
I'm not currently working on this.
Assignee: florian → nobody

Updated

5 years ago
Component: DOM: Device Interfaces → DOM: Contacts
Is this likely to ever happen?
Flags: needinfo?(reuben.bmo)
Nope.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Flags: needinfo?(reuben.bmo)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.