607918 - Native app sharing mime type should not be hard coded text/plain

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Tero Turtiainen]

In bug 601275 the share mime type was hard coded to text/plain. If the mime type is explicitly known, it should be used, otherwise some default could be used.

For example MeeGo offers different set of sharing options depending on the mime type.

Comment 1

[Tero Turtiainen]

Attachment: Patch for fennec

Could the Fennec part be as simple as this?

Then in MeeGo (I can do it) implementation of nsIExternalSharingAppService, if type is null, try to guess it to give the user optimal sharing options. And in Android nsIExternalSharingAppService (somebody else) just set mime type to "test/plain"?

Comment 2

[Tero Turtiainen]

Attachment: MeeGo xulrunner patch

And this might be the patch for MeeGo.

If mime type is not explicitly known, try to guess it. If guessing fails use a default which is text/plain.

Comment 3

[Tero Turtiainen]

Any comments to the patches?

Comment 4

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

Comment on attachment 486599 [details] [diff] [review]
Patch for fennec


>-  show: function show(aURL, aTitle) {
>+  show: function show(aURL, aTitle, aType) {

You added a "aType" param but you never use it. Why?

I assume you want the front-end code to pass the value. Matt Brubeck pointed out, in bug 601275, that Fennec only shares links. Therefore, we never have a need to pass a value for aType.

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

I would be happier with a simple change:

#ifdef MOZ_PLATFORM_MAEMO
    sharingSvc.shareWithDefault(aURL, "text/x-url", aTitle);
#else
    sharingSvc.shareWithDefault(aURL, "text/plain", aTitle);
#endif

Comment 5

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

Or, if really needed _and_ you know it actually works (we can't test), you can add the code removed by bug 601275 back into the MOZ_PLATFORM_MAEMO section

Comment 6

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

Actually, after a little more discussion, we decided to remove the aType parameter from shareWithDefault altogether.

The platform code, as you meego patch shows, can try to guess the type. The android patch would hardcode "text/plain".

Comment 7

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

Comment on attachment 487304 [details] [diff] [review]
MeeGo xulrunner patch

This is very close. Just remove the aMime parameter altogether, from IDL and code, and always try to guess the mime type

Comment 8

[Tero Turtiainen]

(In reply to comment #4)
 >-  show: function show(aURL, aTitle) {
> >+  show: function show(aURL, aTitle, aType) {
> 
> You added a "aType" param but you never use it. Why?

But it is used:
-      this.showSharingUI(aURL, aTitle);
+      this.showSharingUI(aURL, aTitle, aType);

and in showSharingUI:
-    sharingSvc.shareWithDefault(aURL, "text/plain", aTitle);
+    sharingSvc.shareWithDefault(aURL, aType, aTitle);

> I would be happier with a simple change:
> 
> #ifdef MOZ_PLATFORM_MAEMO
>     sharingSvc.shareWithDefault(aURL, "text/x-url", aTitle);
> #else
>     sharingSvc.shareWithDefault(aURL, "text/plain", aTitle);
> #endif

And I would not be happy with that ;-)

> I assume you want the front-end code to pass the value. Matt Brubeck pointed
> out, in bug 601275, that Fennec only shares links. Therefore, we never have a
> need to pass a value for aType.

Is this absolutely certain that Fennec will never in the future share anything but links? What about some extension that may want to share images or othere content? The extension could directly use the nsIExternalSharingAppService but if it was able to use the SharingUI, it would also have the fallback available when there is no implementation for nsIExternalSharingAppService.

And if the aType is removed from shareWithDefault, the extension or future enhancement to Fennec providing some sharing functionality would not even able to explicitly define the mime type and has to rely on the xulrunner guessing the mime type.

Is the mime type guessing able to guess all the video types for example?

For MeeGo, getting the mime type right is important. It decides which sharing options are available for the user. Thats why I would prefer using an explicitly known mime type if available and only then try to guess it.

Comment 9

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

(In reply to comment #8)
> (In reply to comment #4)
>  >-  show: function show(aURL, aTitle) {
> > >+  show: function show(aURL, aTitle, aType) {
> > 
> > You added a "aType" param but you never use it. Why?
> 
> But it is used:
> -      this.showSharingUI(aURL, aTitle);
> +      this.showSharingUI(aURL, aTitle, aType);
> 
> and in showSharingUI:
> -    sharingSvc.shareWithDefault(aURL, "text/plain", aTitle);
> +    sharingSvc.shareWithDefault(aURL, aType, aTitle);

If nothing is ever passed to SharingUI.show(..., aType) then it is never used.

> 
> > I would be happier with a simple change:
> > 
> > #ifdef MOZ_PLATFORM_MAEMO
> >     sharingSvc.shareWithDefault(aURL, "text/x-url", aTitle);
> > #else
> >     sharingSvc.shareWithDefault(aURL, "text/plain", aTitle);
> > #endif
> 
> And I would not be happy with that ;-)

Right. I am now suggesting we remove aType from all code instead.

> > I assume you want the front-end code to pass the value. Matt Brubeck pointed
> > out, in bug 601275, that Fennec only shares links. Therefore, we never have a
> > need to pass a value for aType.
> 
> Is this absolutely certain that Fennec will never in the future share anything
> but links? What about some extension that may want to share images or othere
> content? The extension could directly use the nsIExternalSharingAppService but
> if it was able to use the SharingUI, it would also have the fallback available
> when there is no implementation for nsIExternalSharingAppService.
> 
> And if the aType is removed from shareWithDefault, the extension or future
> enhancement to Fennec providing some sharing functionality would not even able
> to explicitly define the mime type and has to rely on the xulrunner guessing
> the mime type.
> 
> Is the mime type guessing able to guess all the video types for example?
> 
> For MeeGo, getting the mime type right is important. It decides which sharing
> options are available for the user. Thats why I would prefer using an
> explicitly known mime type if available and only then try to guess it.

You obviously believe that knowing the mimetype and passing it explicitly will be a common occurrence, while I do not believe that to be true. I believe that the mimetype will need to be guessed most of the time.

Currently, the nsIExternalSharingAppService interface can only share text-based data. This limits the amount of data types we can actually pass. Given the current interface, how do you imagine we could pass images or videos?

I assume we'd pass links to the images or videos. If the link is a true link, then "text/x-url" is the appropriate guess. If the link is a "data:" URI, then the type can be guessed from the "data:" URI directly. If the link is a "file:" URI, then the type can be guessed from the extension.

Expecting the correct type to be passed explicitly into the interface is a failure IMO.

Comment 10

[Tero Turtiainen]

(In reply to comment #9)
> Currently, the nsIExternalSharingAppService interface can only share text-based
> data. This limits the amount of data types we can actually pass. Given the
> current interface, how do you imagine we could pass images or videos?

Maybe that is the case in Android. I really don't know how sharing works there.

But on MeeGo shareWithDefalt("http://www.example.com/some.jpg", "image/jpeg", "") certainly would give the user the choice of which image sharing options he wants to use. Note, "image/jpeg", not "text/x-url".

Comment 11

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

(In reply to comment #10)

> 
> But on MeeGo shareWithDefalt("http://www.example.com/some.jpg", "image/jpeg",
> "") certainly would give the user the choice of which image sharing options he
> wants to use. Note, "image/jpeg", not "text/x-url".

What about:
shareWithDefault("http://www.example.com/some.asp", "image/jpeg", "")

What happens then, assuming "some.asp" returns an image? What if "some.asp" returns text?

My point is the shareWithDefault implementation will still need to check the values passed in. You just can't blindly trust the parameters.

Comment 12

[Tero Turtiainen]

(In reply to comment #11)
> What about:
> shareWithDefault("http://www.example.com/some.asp", "image/jpeg", "")
> What happens then, assuming "some.asp" returns an image?

User is given the sharing options which are applicable for an image. (would guessing the mime-type from that url succeed?)

> What if "some.asp" returns text?

I guess sharing framework would show an error message.

> My point is the shareWithDefault implementation will still need to check the
> values passed in. You just can't blindly trust the parameters.

Or the sharing framework handles invalid values?

Lets assume three scenarios:
1. http://www.example.com/some.jpg and mime-type text/x-url
2. http://www.example.com/some.jpg and mime-type text/plain
3. http://www.example.com/some.jpg and mime-type image/jpeg

These are three different situations and for each of those, a different set of sharing options is presented to the user. It does not make sense to show any image or link sharing options for "text/plain" and so on.

Guessing the mime type would give pretty good result for http://www.example.com/some.jpg, but would it for http://www.example.com/some.asp if it is an image?

Currently Fennec supports quite primitive sharing. But what about future? What if some extension would want to provide more extensive sharing functionality? The extension may know the actual mime type (link type="some/thing" etc?)?

If the mime type is hard coded, moat of the useful sharing options are not available. If the mime type is guessed, most of the useful sharing options are not available if the guessing is not a trivial operation for a weird url.

So why we should not have the possibility of using an explicitly known mime type?

Comment 13

[BMO Automation]

Closing all opened bug in a graveyard component