[B2G][SMS/MMS] Thumbnails are invisible in the preview attachment with large files



Firefox OS
5 years ago
4 years ago


(Reporter: sarsenyev, Assigned: gnarf)


Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 verified, b2g-v1.1hd fixed)



(2 attachments, 1 obsolete attachment)



5 years ago
Created attachment 765677 [details]

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

Thumbnails are invisible in the preview attachment

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

Repro frequency: 100%
Q Analysts Team Priority: (Pri Level 2)
See the video attachment

See the attachment for video

Comment 1

5 years ago
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
Keywords: qawanted

Comment 2

5 years ago
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
Keywords: qawanted
Kev/Maria, could you clarify what the expectation is for this scenario?
Flags: needinfo?(oteo)
Flags: needinfo?(kev)
Corey, do you know what is happening here? Is related with the size of the files? Thanks!
Flags: needinfo?(gnarf37)
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.
Flags: needinfo?(gnarf37)
maybe the 400kB limit should be put in a setting like the other values ? Or be somewhow calculated like "size limit" * 1.2" ?
Summary: [B2G[Contacts][SMS/MMS] Thumbnails are invisible in the preview attachment with large files → [B2G][SMS/MMS] Thumbnails are invisible in the preview attachment with large files
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!
Flags: needinfo?(gsvelto)

Comment 8

5 years ago
Already blocking on bug 873758 and comment 6 doesn't seem critical for v1.1.
blocking-b2g: leo? → -

Comment 9

5 years ago
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
Assignee: nobody → gnarf37
blocking-b2g: - → leo?

Comment 10

5 years ago
Created attachment 767552 [details] [diff] [review]
patch v1

Uses Settings.mmsSizeLimitation to determine when to render a thumbnail
Attachment #767552 - Flags: review?(felash)


5 years ago
Depends on: 873758


5 years ago

Comment 11

5 years ago
I'm pretty sure this is not the bug that gsvelto is working on so I'm canceling the ni?
Flags: needinfo?(gsvelto)
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
Attachment #767552 - Flags: review?(felash)
triage - agreed to leo+ this per the user impact.
blocking-b2g: leo? → leo+

Comment 14

5 years ago
Created attachment 768125 [details] [diff] [review]
patch v2

* 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;
Attachment #767552 - Attachment is obsolete: true
Attachment #768125 - Flags: review?(felash)
Comment on attachment 768125 [details] [diff] [review]
patch v2

Review of attachment 768125 [details] [diff] [review]:

Attachment #768125 - Flags: review?(felash) → review+

Comment 16

5 years ago
master: https://github.com/mozilla-b2g/gaia/commit/b17b09bf8526be355e2e56a31942c6d9b9d3adab
v1-train: https://github.com/mozilla-b2g/gaia/commit/508cfc8ca95cc5dbfde8dbe8d153ab9653c6ab84
Last Resolved: 5 years ago
status-b2g18: --- → fixed
Resolution: --- → FIXED
v1.1.0hd: 508cfc8ca95cc5dbfde8dbe8d153ab9653c6ab84
status-b2g-v1.1hd: --- → fixed

Comment 19

5 years ago
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.
status-b2g18: fixed → verified


5 years ago
Flags: needinfo?(kev)
Flags: needinfo?(oteo)
You need to log in before you can comment on or make changes to this bug.