Closed Bug 601275 Opened 14 years ago Closed 14 years ago

Native app sharing uses wrong mime-type

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Using Fennec for Android, sharing via native apps is broken in several ways.

Steps to reproduce:
A1. Choose "Share Image" from an image context menu.
A2. Choose Picasa.
Actual results: an error dialog appears: "Upload initialization failed"

B1. Long-tap on a link to an XPI file and choose "Share Link"
Actual results: Only "Email" and "GMail" are listed.  Sharing apps like Twitter are not listed.

C1. Choose "Share Page" from the site menu.
Actual results: Fennec's fallback sharing UI is displayed instead of the native sharing UI.

On Android at least, we should pass "text/plain" if we want to share a URI via an application.  The intent that we send contains the URI as EXTRA_TEXT, not the file the URI points to.  This patch gets rid of the mimeType parameter in the sharing UI code, since the current UI can only share URLs, not files.
Attachment #480284 - Flags: review?(mark.finkle)
tracking-fennec: --- → ?
Blocks: 598269
Attachment #480284 - Flags: review?(mark.finkle) → review+
tracking-fennec: ? → 2.0b2+
Pushed: http://hg.mozilla.org/mobile-browser/rev/bebf8cbee42d

We'll need to see if MeeGo needs any adjustments after this.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
This may actually break things im MeeGo. I thought the idea was that the mime type for shareWithDefault is the mime type of the actual data, not the type of the URI, which is always text. If Android needs the mime type to always be "text/plain" could it just ignore the mime type parameter of shareWithDefault?
Firefox doesn't reliably know the MIME type of the resource that the URI points to.  The previous code was passing "null" for most URIs except already-loaded images.

In fact, the MIME type of the resource may even depends on which app resolves the URI, since it can change based on content negotiation between the client and server.  The only "data" that Fennec is passing to the sharing app is the URI itself.
But completely ignoring the MIME type is not good in my opinion, if the MIME type could be explicitly told to SharingUI. Now you cannot do stuff like SharingUI.show("http://www.example.com", "Here is my link", "text/x-url"); The native sharing application may do things differently for text/x-url and text/plain.

I agree that guessing the mime-type can just go.

But how about something like like this:

showSharingUI: function showSharingUI(aURL, aTitle, aType) {
  let sharinvSvr = Cc[...]
 
  if (!aType)
    aType = "text/plain";

  sharingSvc.shareWithDefault(aURL, aType, aTitle);
}
I would be fine with using "text/x-url" to share URLs on MeeGo, but this is a non-standard MIME type that does not work with existing Android services.  We could make "text/x-url" a default in the MeeGo backend, and "text/plain" a default in the Android backend.  Anyway, can you file a new bug to improve the use MIME type for MeeGo?
(In reply to comment #5)
> I would be fine with using "text/x-url" to share URLs on MeeGo, but this is a
> non-standard MIME type that does not work with existing Android services.  We
> could make "text/x-url" a default in the MeeGo backend, and "text/plain" a
> default in the Android backend.

The point is not to make any MIME type the default on a non-platform specific part of the browser. The point is not that "text/x-url" means anything. The point is that if some MIME type is known, it is bad to override it with "test/plain". For example some extension may know specifically the exact MIME type for the shared stuff and the native sharing application may depend on the exact MIME type. Extension could also use directly nsIExternalSharingAppService but then it would not have the fallback availeble if the sharing app is not available.

Thats why I wouldn't want to make this MeeGo/Android specific but a proper solution should be on the platform independent part. If Android specifically requires "test/plain" THAT should be in the Android part and it should not cause problems for other platforms.

I think the new bug should be about taking some of the code removed on this bug back (guessing the MIME type is not needed) and then in the Android back end override the MIME type with "text/plain". What do you think?
(In reply to comment #6)
> The point is not to make any MIME type the default on a non-platform specific
> part of the browser. The point is not that "text/x-url" means anything. The
> point is that if some MIME type is known, it is bad to override it with
> "test/plain".

If we are going to use a non-standard MIME type for URIs, it should be "text/x-moz-url" which is used in other Gecko code.

> I think the new bug should be about taking some of the code removed on this bug
> back (guessing the MIME type is not needed) and then in the Android back end
> override the MIME type with "text/plain". What do you think?

Agreed, that sounds good.
Also, if the meaning of the URL argument is to describe the resource that the URL points to, then I don't think that "text/x-moz-url" is appropriate (unless the URL points to a resource that just contains another URL).

If we pass "text/html" then the sharing app expects to download a text/html document.  If we pass "text/x-moz-url" does it expect to download a URI?

Since this API *always* takes a URI, adding "text/x-moz-uri" does not add any information.  We can just pass null if we don't know the resource's MIME type.  The back-end can decide how to pass applications a URI of unknown type.
(In reply to comment #8)
> Also, if the meaning of the URL argument is to describe the resource...

Sorry, should be "meaning of the type argument"
Created bug 607918. I'll make a patch sooner or later.
I've performed all reproducing steps from comment #0 on the latest Nightly build and every case has worked as expected. I'll mark this issue as verified fixed.

--
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110912
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: