1095081 - [Gallery]Message prompt to turn on bluetooth appears instead of normal share options when trying to share a photo and video together in the gallery app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect)

Description

[cnelson]

Attachment: log.txt

When the user tries to share a video and photo together in the gallery app, they will notice that a message prompt to turn on bluetooth will appear instead of the default share options. 
   
Repro Steps:
1) Update a Flame device to BuildID: 20141106001204
2) Open the gallery app.
3) Select the square icon on the bottom right.
4) Then select a video and photo.
5) Choose the share icon.
6) Notice a message prompt to turn on the bluetooth appears instead of the normal share options menu.
  
Actual:
Message prompt to turn on bluetooth appears when trying to share a video and photo together in the gallery app.
  
Expected: 
User should see normal share options when trying to share a video and photo together.
  
Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141106001204
Gaia: 9658b93b412bdcc0f953d668e8c8e68318c99fb8
Gecko: 76880403db44
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
  
Repro frequency: 100%
See attached: logcat, video clip https://www.youtube.com/watch?v=E2EsKif1WBk

Comment 1

[cnelson]

This issue does occur on Flame 2.2 Master(319mb)(KitKat)(Shallow Flash), and Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash).

Message prompt to turn on bluetooth appears when trying to share a video and photo together in the gallery app.

Flame 2.2

Device: Flame 2.2 Master(319mb)(KitKat)(Shallow Flash)
Build ID: 20141106043013
Gaia: 068b9711277b06c7d633517f9e1fcb5624bb39b3
Gecko: 41adad987d82
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141105183012
Gaia: 5ee26701a4d8db266bfb203b2179f686ce14d8b6
Gecko: dbf49343e889
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This seems like it could be a blocker, or by design. NI Gallery owner to give more info

Comment 3

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:
In this case, option to send it via email should definitely be displayed, but it skips directly to bluetooth transfer.  I believe this is a blocker that needs to be fixed in 2.0, 2.1, and 2.2.  I marked it as 2.1 blocker for now.  Can devs comment on this?

Comment 4

[Hema Koka [:hema]]

I don't recall that the behavior seen is by design when video and images are picked for sharing. Seems like broken function. Adding David for input.

Comment 5

[David Flanagan [:djf]]

I agree that we should fix this, but its not exactly a bug, just a case that we haven't figured out how to handle yet.

Most apps only share one type of data, and specify that type in the activity request. Apps that know how to handle that type of data register activity handlers for that type. An app that can handle images (like the wallpaper app) might not be able to handle videos.  

The bluetooth app doesn't care about types. You can share any kind of blob with it and it will transfer it.

Maybe the email app should be the same way. As it stands, however, the email app is set up to only handle shares of one type of blob at a time. You can select multiple images and share those via email. Or multiple videos and share those.  

In gallery, if you select one photo and one video and try to share, the type associated with the activity will be "multipart/mixed".  This seems like a smart thing to do, but it is also a thing that I just made up.  The problem is that there are no apps (except bluetooth transfer which just ignores the type) that actually know how to handle multipart/mixed.

The easiest way to fix this is to ask Andrew to modify the email app to accept multipart/mixed share activities.  Setting needinfo for him.  Can you do that, Andrew? Or is there some other way you'd prefer to handle share activities where the blobs aren't all of the same type. I suppose we could also define some specific type like 'image/*,video/*' and use that in both gallery and email.

I should also try making Gallery just set an array of ['image/*', 'video/*'] to see if that works with email. According to the activity docs on MDN, that won't work, because gallery must provide a single value that exists in emails array of accepted values. But if MDN is wrong, then there is an even simpler fix in gallery.

Comment 6

[David Flanagan [:djf]]

So the problem with having gallery declare the type of the share as an array of mime types is that the way activity filters work, if any of the values gallery specify match any of the values the app specifies, then that is considered a match. So if I make that change in gallery, then email is a choice when I share an image and a video.  But so is the test "share receiver" app, which expects only images and doesn't know how to handle videos.  

Doing that also allows Messages to be shown as one of the sharing choices. But it crashes when I try to give it both an image and a video that way. Ideally, the messages app would also be able to handle type="multipart/mixed". But that would be a separate bug.

Maybe better than multipart/mixed would be something a little more explicit like 'multipart/mixed;image,video' or something like that. If Andrew can make the email app handle that type, I can make gallery use that type.

Another solution would be for gallery to just display an error message if the user tries to share things of different types, but that only seems like it would be worth doing for 2.1. For master, we should fix this correctly.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

We should stay away from multipart/blah and friends.  Although MIME media types of the form "type/subtype" (http://tools.ietf.org/html/rfc2046) have managed to escape from the RFC (2)822 encoding used by email, the spec for "multipart" (http://tools.ietf.org/html/rfc2046#section-5.1) is strongly bound to the text encoding with text boundary delimiters and all that sadness (http://tools.ietf.org/html/rfc2045).  For example, multipart/form-data for HTTP POST.

(It would also be especially confusing semantically for email, since technically if you are trying to share a multipart/mixed thing with us, it would most logically make sense for it to be a fully encoded MIME text blob.)


This issue came up on bug 848400 and I tried to start a thread at https://groups.google.com/d/msg/mozilla.dev.gaia/Dp1fbH9-Uh4/LCEmY_1pyOkJ that never quite got a lot of love.

What we ended up with on that bug is deciding that "application/*" means "I don't care what it is, I can attach anything", with the rationale stemming from the existence of "application/octet-stream" (also defined in those RFC's!!) as "used to indicate that a body contains arbitrary binary data" per http://tools.ietf.org/html/rfc2046#section-4.5.1 and consistent with our existing use of "/*" and some existing code that also just suffixes stuff like that.  I think this approach makes sense.

From a web platform perspective, the actual MIME type should theoretically not particularly matter since <img>/<audio>/<video> should have the same support across all the apps on a device or any web services they'd interact with.  And if we're just dealing with opaque binaries "application/*" or "application/octet-stream" does cover it.

From a UX perspective, I think this also makes sense.  soundcloud only cares about "audio/*", youtube only cares about "video/*", flickr cares about "image/*" and "video/*", dropbox doesn't care and so "application/*" works for it, etc.  The question of multiplicity potentially matters, so in theory it would make sense to expose blobCount as something that can be used to make sure only 1 blob is received.  In this it arguably makes sense to use a type hierarchy such that if you mix things, the MIME type becomes "application/*".


The email app has cared up to this point only because there originally was no download manager and so anything other than audio/video/image would get lost on the device given how we implemented attachment handling.


Having said all that, there's an effort to standardize a sharing API at https://wiki.whatwg.org/wiki/Sharing and ideally we'd just do what that wants longer term.

For now I'd propose something like this:
- We stick to audio/*, image/*, video/*, and application/* with the type hierarchy rules we promote to application/* if we mix things.
- For v2.2 email will support application/*

This poses a problem for sites like flickr that might want to only support image/* and video/* but do want to support them being shared at the same time.  Now we get into the issue of backwards compatibility.  This strategy is conservative, but there's nothing to stop flickr from supporting "application/*" and informing the user when it sees a Blob in the list that isn't a MIME type it actually supports.


The alternative case where gallery is basically summarizing the MIME types included for ['image/*', 'video/*'] arguably makes it impossible for a picture sharing site or video sharing site to avoid getting the MIME type it doesn't want.  We didn't document or define any semantics for "share", so we're all in the wrong whatever happens there.

Comment 8

[David Flanagan [:djf]]

Okay, so for 2.2, I'll modify Gallery to use application/* for mixed types and Andrew will modify Email to accept that type. (I've filed bug 1097117 for the Email app change and bug 1097122 for a corresponding change to Messages.)

We're left with the question of what to do in 2.1, and this is complicated by the fact that it is too late for string changes, so we can't do anything like put up a dialog saying "if you try to share mixed batches, you may only get bluetooth". Here are some possibilities

1) Do nothing. This bug has not been triaged yet, but maybe it does not need to be fixed.

2) Add something to the activities api so that when the gallery launches the activity it can specify that it wants the user to see the menu of app choices even when there is only one possible app.  Currently a large part of the user confusion is that the bluetooth app just gets launched without the user choosing it. If the user saw that bluetooth was the only available sharing option and could press cancel instead, that would change things a lot. (I've filed bug 1097129 for this because we should do this anyway, even if we fix this bug some other way.)

3) Modify the gallery app so that if the current selection contains both images and videos then the share button is disabled.  It would be better to leave it enabled and display a warning when the user presses share in this case, but we can't do that because of string freeze.  This is easy but hard for the user to understand.

If we decide to make this bug a 2.1 blocker, we'll need UX advice on whether to go with option #2 or #3.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Thanks for filing the bug!

Yeah, the lack of a menu can be a real hassle.  Email is the only handler of share with type "url", or has been in the past, and we ran into edge-cases where the user managed to tap on the link two or more times before the email app popped up.  And then the email app was faced with two or more compose activities in a row.  It might be less horrible for apps that use an inline disposition rather than a window disposition.

I mention that because when pondering v2.1 mitigations that might be a reason to just always bring up the menu, even without adding a weird flag.  (Which would want to happen on trunk, too.)

Comment 10

[Hema Koka [:hema]]

Moving out of 2.1 nom and into priority backlog.

Comment 11

[David Flanagan [:djf]]

> 
> I mention that because when pondering v2.1 mitigations that might be a
> reason to just always bring up the menu, even without adding a weird flag. 
> (Which would want to happen on trunk, too.)

If we did that, we'd break things like the switch from gallery to camera and back. And some of the activties that are used to launch the settings app.  So I think it really needs to be opt-in. Though I can see a case for allowing either the initiating app or the handling app to force a menu display. But I don't think we can safely make it the default.

Comment 12

[David Flanagan [:djf]]

Looks like I completely dropped the ball on this. Andrew did what was agreed in comment #8, but I never made the required change for gallery, and this is something that we wanted to fix for 2.2. I think it is just a one-line change at gallery.js:651 to change multipart/mixed to application/*

Punam: if you have time to make that gallery.js change on Friday and test it carefully, maybe we can still get this fix uplifted into 2.2.  Are you able to work on it?

Comment 13

[Punam Dahiya [:pdahiya]]

(In reply to David Flanagan [:djf] from comment #12)
> Looks like I completely dropped the ball on this. Andrew did what was agreed
> in comment #8, but I never made the required change for gallery, and this is
> something that we wanted to fix for 2.2. I think it is just a one-line
> change at gallery.js:651 to change multipart/mixed to application/*
> 
> Punam: if you have time to make that gallery.js change on Friday and test it
> carefully, maybe we can still get this fix uplifted into 2.2.  Are you able
> to work on it?

Sure, will test and submit the patch for review.

Comment 14

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1095081 > mozilla-b2g:master

Comment 15

[Punam Dahiya [:pdahiya]]

Comment on attachment 8588092 [details] [review]
[gaia] punamdahiya:Bug1095081 > mozilla-b2g:master

Hi David

Tested and with the patch user sees the menu option to choose email, bluetooth to share images and videos selected together. Both the flows (email, bluetooth) successfully shares the selected files. Please review. Thanks!

Comment 16

[David Flanagan [:djf]]

Comment on attachment 8588092 [details] [review]
[gaia] punamdahiya:Bug1095081 > mozilla-b2g:master

Thanks, Punam. I have tested as well and can confirm that I can select a photo and a video and send both via bluetooth or email.

r+ to land this on master.

And we should also try to get this uplifted to 2.2. I'll request that.

Comment 17

[David Flanagan [:djf]]

Comment on attachment 8588092 [details] [review]
[gaia] punamdahiya:Bug1095081 > mozilla-b2g:master

Bhavana: this is a trivial patch that should have gone into 2.2 months ago, but did not because I forgot about it. So now requesting uplift approval at the last minute.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): not a regression

[User impact] if declined: if the user selects a photo and video in gallery and tries to share them, the email app will not be a choice. Bluetooth will be a choice, but they may be very confused because since there is only one sharing choice, they will be taken directly to the bluetooth sharing app, which is probably not what they expect. It is pretty bad UX.

[Testing completed]: yes, locally

[Risk to taking this patch] (and alternatives if risky): Not risky. It is a one-line change that only affects relatively unusual share activity requests. Those requests are currently badly broken, so there is no way that this patch could make anything worse than it is now. All we do is change the MIME type of the share request from multipart/mixed to application/* for compatibility with the email app.

[String changes made]: none

Comment 18

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/1c1eb64260e8f8e08db0e58c54565b23b7a327ac

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a6351e1197d54f8624523c2db9ba1418f2aa046f

Comment 20

[Lancy(Leave from Mozilla)]

Attachment: Verify1_v2.2&Master_Pass.3gp

This issue is verified as pass on latest Flame v2.2/3.0 and N5 v2.2/3.0 build by the STR in comment 0.

Actual Result:The share options of email and bluetooth are displayed when trying to share a video and photo together.

See attachment:Verify1_v2.2&Master_Pass.3gp.
Reproducing rate:0/10

Device: Flame 2.2 build (Pass)
Build ID               20150629162503
Gaia Revision          b39d4f5b4937592ded19ec65e113a74177ae1f86
Gaia Date              2015-06-29 13:01:35
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cefa70ef71e4
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150629.223916
Firmware Date          Mon Jun 29 22:39:27 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 build (Pass)
Build ID               20150629134017
Gaia Revision          27fe0f4261e3685187769411f2f74cff19287b19
Gaia Date              2015-06-29 14:29:00
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/c26dbd63604d
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150629.170951
Firmware Date          Mon Jun 29 17:10:03 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5_2.2 build (Pass)
Build ID               20150629162503
Gaia Revision          b39d4f5b4937592ded19ec65e113a74177ae1f86
Gaia Date              2015-06-29 13:01:35
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cefa70ef71e4
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150629.223845
Firmware Date          Mon Jun 29 22:39:02 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5_3.0 build (Pass)
Build ID               20150629134017
Gaia Revision          27fe0f4261e3685187769411f2f74cff19287b19
Gaia Date              2015-06-29 14:29:00
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/c26dbd63604d
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150629.170843
Firmware Date          Mon Jun 29 17:09:00 EDT 2015
Bootloader             HHZ12f