971987 - Evaluate whether to continue using social API for client

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Comment 1

[Mark Banner (:standard8)]

Attachment: Let Social API load chrome uris (moved to bug 976614)

This does enough to let Social API load chrome:// uris.

Loading as chrome uris also gives access to Components.classes which I think should be enough to access what we need.

The patch isn't quite complete - it doesn't work in the debugging case where social.debug.injectIntoTabs is set to true, and you're loading the panel in a tab. I know roughly what's wrong, just need to fix it.

We'll need to verify that the social guys are happy with this - I'll talk to Shane next week.

I'll attach a demo in a few.

Comment 2

[Mark Banner (:standard8)]

Attachment: Chrome uri based demo

This is the chrome uri demo. It alters the necessary preferences, and creates a new directory under browser/components.

The panel has a link where you can access the social api and create a panel, plus it also logs the app name via using access to the nsIXULAppInfo service which proves access to Components.classes etc, which I think will mean we have enough to handle push notifications appropriately.

Comment 3

[Mark Banner (:standard8)]

Attachment: Chrome uri based demo with module (moved to bug 976358)

Updated the demo just to use a module. Hence proving that we don't necessarily need a worker (or the social api worker), and that we can access gecko just fine.

Comment 4

[Adam Roach [:abr]]

Now that the technical viability is demonstrated (assuming the patch can be approved and landed), I think it's safe to assume that we can use this through MLP at least. I'm taking ownership of this bug to evaluate the non-technical (e.g., development) viability of this API long-term.

Comment 5

[Shane Caraveo (:mixedpuppy)]

I'm on PTO, but this seems like a prompt response would be useful, and it is a quick one so I may be missing some nuance, lets talk later (I'm back to work the 26th).

I'd be concerned about what other social providers might be able to do since this patch essentially allows any provider to stick a chrome url in their manifest.  At a bare minimum I'd like to see a social.chrome.whitelist pref, similar to social.whitelist, that restricts what providers can use chrome URIs.  

IMO permissions could be handled differently without chrome access, and wrapper APIs could be used for access to specific internal APIs (e.g. getting contacts).   I'm not clear how chrome access gives you the ability to bypass the permissions dialogs (I'm guessing there is a way), and I am less clear whether that is desirable.  The sample usage patch doesn't show anything obvious that couldn't be done already.

This feels like it is bypassing one of the primary tenants of socialapi, which is no direct access to Firefox internals, I'd like to understand what is going on in more detail.  

Also, ISTM that these patches should be in their own bug, not a meta bug, or that the title of this bug is not correct.

In any case, opening chrome access to social providers is something that someone on the Firefox team should also chime in on.

Comment 6

[Mark Banner (:standard8)]

I think one of the issues here, is that we want to host the files in chrome, under chrome uris rather than loading them remote. That immediately means we have chrome access, although Adam is looking at ways that some of this can operate in non-chrome context.

Comment 7

[Shane Caraveo (:mixedpuppy)]

another issue that would need to be addressed, ensuring that the provider cannot be removed via about:addons.

Comment 8

[Florian Quèze [:florian]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #5)

> I'd be concerned about what other social providers might be able to do since
> this patch essentially allows any provider to stick a chrome url in their
> manifest.

I think we could check at install time that a social provider doesn't include a chrome: url in its manifest.

The specific social provider we are interested in here would ship by default within Firefox, so it won't be 'installed' by the user.

> I'm not clear how chrome access gives you the ability
> to bypass the permissions dialogs (I'm guessing there is a way)

For getUserMedia, chrome code doesn't need the prompt at all:
http://hg.mozilla.org/mozilla-central/annotate/1422dfcd7fd8/dom/media/MediaManager.cpp#l1418

> This feels like it is bypassing one of the primary tenants of socialapi,
> which is no direct access to Firefox internals, I'd like to understand what
> is going on in more detail.  

Some of what's going on here: we are no longer trying to build a webapp, and instead we are working on something that could ship within Firefox by default. So this is no longer really a 'social provider', but it still makes sense to reuse the existing UI that has been developed for SocialAPI, as the UI needs are very similar.

> Also, ISTM that these patches should be in their own bug, not a meta bug, or
> that the title of this bug is not correct.

I think the title of the bug is correct, and we (more specifically Adam and Mark) are just working on evaluating what's possible, and how many changes are required.

> In any case, opening chrome access to social providers

I don't think this is what's suggested here.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Florian Quèze [:florian] [:flo] from comment #8)
> 
> Some of what's going on here: we are no longer trying to build a webapp, and
> instead we are working on something that could ship within Firefox by
> default. So this is no longer really a 'social provider', but it still makes
> sense to reuse the existing UI that has been developed for SocialAPI, as the
> UI needs are very similar.
>

While this is true, one bit of nuance is that we're currently hoping to share at least one piece (the code running in the chatwindow) with both other modern web-browsers (1st target is likely chrome), and later, ideally, a b2g app.  How practical this hope will turn out to be is not yet clear...

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8379831 [details] [diff] [review]
Let Social API load chrome uris (moved to bug 976614)

This has moved to bug 976614.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8380577 [details] [diff] [review]
Chrome uri based demo with module (moved to bug 976358)

This has moved to bug 976358.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Current belief based on discussion with Adam and others is that this is a reasonable choice for MLP; resolving.

Comment 13

[u279076]

This does not appear to be something that needs QA attention before we release. Please needinfo me to request specific QA testing.