Description: After attaching 3 large(more than 1mb) picture files to "MMS", the attached files are invisible in thumbnail view Repro Steps: 1) Updated to Buri/Inari/Leo/Unagi Build ID: 20130610070206 2) Open "SMS" app from the home menu 3) Tap on a new message (paper and pen) icon 4) Tap on "Clip" icon in the left corner 5) Choose: 6) Select from "Gallery" 7) Choose a picture after cropping (checkbox) icon 8) Repeat steps 3-7 two more times until the three files appear in the attachment Actual: Thumbnails are invisible in the preview attachment Expected: A user can see all pictures in preview mode. Environmental Variables Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8e3f39363c54 Gaia: ce3b99781d182ad550a325206990c249b0dbcf0e Platform Version: 18.0 Notes: Repro frequency: 100% Q Analysts Team Priority: (Pri Level 2) See the video attachment See the attachment for video

Adding qawanted to find out if this is caused by 1) Large images (as in images not taken with the phone's camera) 2) Multiple images being attached (how many?) 3) A combination of the two

1) Yes, images are not taken with the camera, just transferred from PC 2) Three images being attached (see the video attachment) 3) Combination of the three

Kev/Maria, could you clarify what the expectation is for this scenario?

Corey, do you know what is happening here? Is related with the size of the files? Thanks!

This is expected, we've put an hard limit of 400kB for generating thumbnails, because we don't want the app to crash. But soon (bug 873758) we should recompress images before adding them as attachment, and the goal size is 300kB for this, which is less than 400kB (obviously ;) ), and therefore this should just work.

maybe the 400kB limit should be put in a setting like the other values ? Or be somewhow calculated like "size limit" * 1.2" ?

Gabriele, Is this the bug you are working on? I've seen that is not assigned and I dont know if is the same. Thanks!

Already blocking on bug 873758 and comment 6 doesn't seem critical for v1.1.

Alex - This becomes critical if the operator defined limit is say 1mb, 400k is easily one image and therefore won't have any previews. operatorLimit * 1.2 makes sense to me julien, I'll write a quick patch for this hoping that it gets leo+'ed

Uses Settings.mmsSizeLimitation to determine when to render a thumbnail

I'm pretty sure this is not the bug that gsvelto is working on so I'm canceling the ni?

Comment on attachment 767552 [details] [diff] [review] patch v1 Review of attachment 767552 [details] [diff] [review]: ----------------------------------------------------------------- looks good ! some requested changes still ;) ::: apps/sms/js/attachment.js @@ +163,5 @@ > // We special case audio to display an image of an audio attachment video > // currently falls through this path too, we should revisit this with > // Bug 869244 - [MMS] 'Thumbnail'/'Poster' in video attachment is needed. > + if (type === 'img' && (!Settings.mmsSizeLimitation || > + this.size < Settings.mmsSizeLimitation * 1.2)) { I'd say we should keep the MAX_THUMBNAIL_GENERATION_SIZE in case we don't have Settings.mmsSizeLimitation. Maybe rename that constant. I also still think we should still have a sensible absolute maximum, but I guess this should be configurable per device too (because it's probably "available memory dependent"), so we might have this in another bug. ::: apps/sms/test/unit/attachment_test.js @@ +113,5 @@ > name: 'Image attachment' > }); > var el = attachment.render(function() { > assert.ok(el.src, 'src set'); > assert.include(el.classList, 'attachment'); I know newer chaijs don't like using include with classList (and that's part of why we can't upgrade to the newer chaijs). Can you use "assert.ok(el.classList.contains(...))" instead ? Its not less readable anyway. @@ +129,5 @@ > + name: 'Image attachment' > + }); > + var el = attachment.render(function() { > + assert.ok(el.src, 'src set'); > + assert.include(el.classList, 'attachment'); same here

triage - agreed to leo+ this per the user impact.

* Sets max thumbnail generation size to 1.5 megs, which should be big enough for any mms image (instead of using max mms size) * Removes a var self = this;

Comment on attachment 768125 [details] [diff] [review] patch v2 Review of attachment 768125 [details] [diff] [review]: ----------------------------------------------------------------- r=me

master: https://github.com/mozilla-b2g/gaia/commit/b17b09bf8526be355e2e56a31942c6d9b9d3adab v1-train: https://github.com/mozilla-b2g/gaia/commit/508cfc8ca95cc5dbfde8dbe8d153ab9653c6ab84

v1.1.0hd: 508cfc8ca95cc5dbfde8dbe8d153ab9653c6ab84

Varified,fixed on leo 1.1 Mozilla RIL Environmental Variables Build ID: 20130717070237 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/582e3a7018b0 Gaia: c506c50adaaebcf729ac3c27887ba2931ab79040 Platform Version: 18.1 Issue does not reproduce user can see all pictures in preview mode.