851098 - Enable mozContacts on Firefox desktop

Status: RESOLVED WONTFIX

(Core Graveyard :: DOM: Contacts, defect)

Description

[Florian Quèze [:florian]]

Attachment: 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.

Comment 1

[Mounir Lamouri (:mounir)]

To what kind of content would that be available?

Comment 2

[Florian Quèze [:florian]]

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

Comment 3

[[:fabrice] Fabrice Desré]

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

Comment 4

[Mounir Lamouri (:mounir)]

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

Comment 5

[Florian Quèze [:florian]]

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

Comment 6

[Florian Quèze [:florian]]

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

Comment 7

[[:fabrice] Fabrice Desré]

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

Comment 8

[Florian Quèze [:florian]]

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

Comment 9

[[:fabrice] Fabrice Desré]

My point is not that your code does not work, but that this should not be done this way.

Comment 10

[Mounir Lamouri (:mounir)]

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

[Gregor Wagner [:gwagner]]

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.

Comment 12

[Reuben Morais [:reuben]]

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.

Comment 13

[Kevin Grandon :kgrandon]

FYI - we currently import these for DEBUG gaia builds running inside of firefox, so the contacts API should work there.

Comment 14

[Florian Quèze [:florian]]

I'm not currently working on this.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Is this likely to ever happen?

Comment 16

[Reuben Morais [:reuben]]

Nope.