880825 - [MMS] Unable to Open audio and video immediately after attaching

Status: RESOLVED WORKSFORME

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

Description

[Mike Pennisi [:jugglinmike]]

Steps to reproduce:

1. Open an existing message thread
2. Attach an audio or video file to the new message
3. Click on the attachment's placeholder in the message composition field
4. Select "View"

Expected: the media opens correctly in the appropriate application

Actual: the media viewing application opens and crashes without playing back the media

After sending the message, the "view" behavior can be triggered by clicking on the attachment placeholder in the Thread UI. Here again, the viewing application crashes.

The expected behavior is demonstrated only after navigating away from the Thread UI (to the Thread List, for example), re-opening the message thread again, and attempting to view the attachment again.

Comment 1

[Dietrich Ayala (:dietrich)]

what are the audio/video types causing this problem?

Comment 2

[Corey Frang [:gnarf]]

as far as we could tell, everything we had on the device

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

As far as I can tell the problem here is very similar to bug 877482. The SMS app is sending an activity with the filename it wants to attach and a File object as the blob; the Music app is interpreting the blob as raw data instead which leads to the crash. I'm preparing a patch to fix the issue, hopefully it shouldn't be too complicated.

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Handle file blobs correctly

This is a tentative patch for fixing the bug, I did some light testing with it and it seems to work correctly. The patch applies properly to v1-train too and I'll attach a PR shortly.

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/10604

Pointer to Github pull-request

Comment 6

[Dominic Kuo [:dkuo]]

Comment on attachment 767145 [details] [diff] [review]
[PATCH] Handle file blobs correctly

Review of attachment 767145 [details] [diff] [review]:
-----------------------------------------------------------------

Gabriele, I took a quick look on your patch before I test it, and I found it's a same workaround which I removed in bug 867448 comment 7, I believe this should be a gecko issue and a gaia workaround probably can fix this, so if we are trying to meet the schedule or we better to wait for the gecko patch.

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Dominic Kuo [:dkuo] from comment #6)
> Gabriele, I took a quick look on your patch before I test it, and I found
> it's a same workaround which I removed in bug 867448 comment 7, I believe
> this should be a gecko issue and a gaia workaround probably can fix this, so
> if we are trying to meet the schedule or we better to wait for the gecko
> patch.

I agree, the Gecko patch for b2g18 is in bug 887093; I would also want to go for it as long as we can land it before the end of the week due to the constraints we have for leo+ bugs. If we do land it then we don't need any changes in the Music or Video apps and we just need to get rid of the existing Gallery workaround (see attachment 767587 [details] [diff] [review]).

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

I've retested now that the fix for bug 887093 and the problem is fixed on v1-train; on master on the other hand the music app does not crash but does not play the attached sound either. I will need to further investigate.

Comment 9

[Dominic Kuo [:dkuo]]

(In reply to Gabriele Svelto [:gsvelto] from comment #8)
> I've retested now that the fix for bug 887093 and the problem is fixed on
> v1-train; on master on the other hand the music app does not crash but does
> not play the attached sound either. I will need to further investigate.

What format you are testing?

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

I've just re-tested this and it seems to work fine on master too. I've looked into the log to see if the fix for bug 887093 was ported over but couldn't find it. Ben can you confirm that you fixed the blob problem in bug 887093 on m-c? If that's the case I'd like to close this with a reference to the relevant change.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

No, bug 887093 was a b2g18-only fix (the blob architecture on trunk is totally different from b2g18).

Comment 12

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to ben turner [:bent] (needinfo? encouraged) from comment #11)
> No, bug 887093 was a b2g18-only fix (the blob architecture on trunk is
> totally different from b2g18).

I see... would that bug still be present on central then or would central not suffer from the problem at all? If it would then I have to look at the Gaia sources to see if a workaround has been put in place instead.

Comment 13

[Julien Wajsberg [:julienw]]

There were several fixed with crashes over blob, will be difficult to find the correct change.

Let's close :)

Comment 14

[Julien Wajsberg [:julienw]]

(my english is worse than ever)