598269 - Rewrite sharing front end to use the sharing service

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Tero Turtiainen]

Rewrite sharing front end to use platform specific sharing services.

Specification at http://etherpad.mozilla.com:9000/sharing-interface

Comment 1

[Matt Brubeck (:mbrubeck)]

Proposed behavior:

* If getSharingApps returns multiple apps (expected on Android), then Fennec will ask the user to choose an app.

* If getSharingApps returns one app (expected on MeeGo), then Fennec will immediately launch that app.  The default sharing app on MeeGo will display its own chooser UI.

* If getSharingApps returns zero results (expected on Maemo 5 and on desktop platforms), then Fennec will fall back to a list of hard-coded in-browser sharing services (currently Email, Facebook, Twitter, and Google).

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #1)
> Proposed behavior:
> 
> * If getSharingApps returns multiple apps (expected on Android), then Fennec
> will ask the user to choose an app.
> 
> * If getSharingApps returns one app (expected on MeeGo), then Fennec will
> immediately launch that app.  The default sharing app on MeeGo will display its
> own chooser UI.
> 
> * If getSharingApps returns zero results (expected on Maemo 5 and on desktop
> platforms), then Fennec will fall back to a list of hard-coded in-browser
> sharing services (currently Email, Facebook, Twitter, and Google).

getSharingApps will not be implemented in the first phase of this project. Only shareWithDefault will be implemented right away (for beta 2 at least).

See bug 598268

If getSharingApps fails, say on the N900, the original design was to fallback to email and sms only.

See bug 596632

Comment 3

[Tero Turtiainen]

Attachment: Patch

Patch for fennec which uses the proposed implementation in bug 598268.

Basically, adds to SharingUI a method for calling shareWithDefault of nsIExternalSharingAppService. If it fails, falls back to the current sharing user interface implementation.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 477463 [details] [diff] [review]
Patch

This is a good patch. The only reason I don't want to r+ it yet is that we need to settle on the backend service.

The Android patch and the Meego patch are slightly different in how the service would be accessed.

Cc["@mozilla.org/uriloader/external-protocol-service;1"]       .getService(Ci.nsIExternalSharingAppService);

 versus

Cc["@mozilla.org/uriloader/external-sharing-app-service;1"]       .getService(Ci.nsIExternalSharingAppService);

I'll check the status of the Android patch. I think I might like the "external-sharing-app-service;1" approach a little more.

Nice work though!

Comment 5

[Tero Turtiainen]

Once the backend method is settled, I can provide a modified patch (well, the service name is changed plus any additional fixes needed). I now have a work-in-progress version of the MeeGo backend which works with the Android patch.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 477463 [details] [diff] [review]
Patch

>diff -r ea70f3df7037 chrome/content/browser-ui.js

>+  showSharingUI: function showSharingUI(aURL, aTitle, aType) {
>+    let sharingSvc = Cc["@mozilla.org/uriloader/external-protocol-service;1"]
>+                         .getService(Ci.nsIExternalSharingAppService);

The final contract is:

"@mozilla.org/uriloader/external-sharing-app-service;1"

I can change that when I land the patch

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/7b738fe45c1a

Comment 8

[Cristian Nicolae (:xti)]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110913
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2