875619 - [MMS] Can't retrieve attachment sent from iPhone

Status: VERIFIED FIXED

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

Description

[Paul Yang [:pyang] (away)]

iPhone attachment can't be retrieved and icons remain blank.

REPRODUCE STEPS:
1. Use iPhone to send an image attached MMS to Fxos device.
2. Open message app, switch to target message thread.

EXPECTED:
1. Attachment preview is shown correctly.

ACTUAL:
1. No attachment is shown, icon is blank.

Comment 1

[Corey Frang [:gnarf]]

Attachment: patch v1

Reason: iPhone doesn't include a filename for its attachments

Comment 2

[Steve Chung [:steveck]]

Comment on attachment 753602 [details] [diff] [review]
patch v1

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

::: apps/sms/test/unit/smil_test.js
@@ +108,5 @@
>        SMIL.parse(message, function(output) {
>          assert.equal(output[0].text, testText);
>          assert.equal(output[0].blob, testImageBlob);
> +        // in lieu of location, it uses id stripped of unsafe chars
> +        assert.equal(output[0].name, '23');

Why not having another test case like "attachment with id and content only"? Maybe it would be better that comment about which device will send mms like this kind of format.

@@ +175,5 @@
>        SMIL.parse(message, function(output) {
>          assert.equal(output[0].text, testText);
>          assert.equal(output[0].blob, testImageBlob);
> +        // uses id in lieu of location
> +        assert.equal(output[0].name, '2');

Same here: We can have another test case here too.

Comment 3

[Corey Frang [:gnarf]]

Attachment: patch v2

reworked tests

Comment 4

[Corey Frang [:gnarf]]

+2 typo fixes: https://gist.github.com/gnarf37/55e98ae2afb0a44cb511 (bit rushed)

Comment 5

[Steve Chung [:steveck]]

Comment on attachment 753608 [details] [diff] [review]
patch v2

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

Since these typo are test only, r+ with these typo fixing. Thanks.

::: apps/sms/test/unit/smil_test.js
@@ +130,5 @@
> +      SMIL.parse(message, function(output) {
> +        assert.equal(output[0].text, testText);
> +        assert.equal(output[0].blob, testImageBlob);
> +        // uses the id
> +        assert.equal(output[0].name, '2');

output[0].name should be 23 here

@@ +190,5 @@
>            location: 'text1',
>            content: new Blob([testText], {type: 'text/plain'})
>          },{
>            id: '<2>',
> +          location: 'example.jpg'

Comma missed here.

Comment 6

[Corey Frang [:gnarf]]

master: aa34feca04eb51e5ab98a38b8fce2f23b6492a5d

Comment 7

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 85c558e

Comment 8

[Paul Yang [:pyang] (away)]

Verified on v1-train, bug fixed