949202 - [MMS] Unable to view specific small pictures from MMS attachment taken from camera

Status: RESOLVED FIXED

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

Description

[Jason Smith [:jsmith]]

Followup to bug 944276. The fix there was a Gaia workaround for 1.2 specifically. This bug tracks doing the real fix for 1.3+.

Comment 1

[David Flanagan [:djf]]

Kyle,

Jason closed your koi+ bug and filed this new one for the 1.3 fix, but didn't assign you to it. Just setting needinfo to make sure it stays on your radar.

Comment 3

[Julien Wajsberg [:julienw]]

Hey Kyle, do you think you'll be able to have a look soon? Or should we apply bug 944276 in 1.3 too?

Comment 4

[Peter Dolanjski [:pdol]]

PM retriaged this and we agree with the blocking status.

Comment 5

[Kai-Chih Hu [:khu]]

(In reply to Julien Wajsberg [:julienw] from comment #3)
> Hey Kyle, do you think you'll be able to have a look soon? Or should we
> apply bug 944276 in 1.3 too?

I was thinking that, how about doing testing 1.3 with the patch from bug 944276? If it can fix this issue, maybe we can apply the fix to 1.3. How is that?

Comment 6

[Jason Smith [:jsmith]]

(In reply to Kevin Hu [:khu] from comment #5)
> (In reply to Julien Wajsberg [:julienw] from comment #3)
> > Hey Kyle, do you think you'll be able to have a look soon? Or should we
> > apply bug 944276 in 1.3 too?
> 
> I was thinking that, how about doing testing 1.3 with the patch from bug
> 944276? If it can fix this issue, maybe we can apply the fix to 1.3. How is
> that?

There isn't value to pursue this from the QA side. The solution in bug 944276 targets a 1.2-specific solution off the 1.2 branch. This bug focuses on providing the real solution. If we're to use the workaround again, then we would need a 1.3 patch on the 1.3 branch, which is best solved by a developer.

Comment 7

[Hema Koka [:hema]]

Kyle: Would be able to take this on? We are shooting to get 1.3+ bugs fixed as soon as possible (by 1/31)

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Is there any reason we can't apply the workaround again.  I'm at a work week this week so my time is rather limited.

Comment 9

[David Flanagan [:djf]]

Attachment: 1.2 workaround cherry-picked into the v1.3 branch

We really *ought* to have a gecko fix for this, and it is frustrating that we haven't gotten any traction on it from the gecko developers.  On the other hand, it has only been 6 weeks or so since this was a 1.2 blocker.

I don't see any reason why we can't apply the workaround again.

I've cherry picked the commit. It applied cleanly to the 1.3 branch.  I'm attaching it here, but note that I haven't even tested it.  If Julien approves of using it again, maybe QA can verify that it still works to fix the bug.

Comment 10

[Julien Wajsberg [:julienw]]

I'd rather fire a new 1.3+ bug for this and make this one 1.4+.

Comment 11

[Julien Wajsberg [:julienw]]

Comment on attachment 8367676 [details] [review]
1.2 workaround cherry-picked into the v1.3 branch

r=me, fixes the issue (I tried it myself)

but please land it in a new bug, let's keep this one for the Gecko issue.

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

> We really *ought* to have a gecko fix for this, and it is frustrating that
> we haven't gotten any traction on it from the gecko developers.  On the
> other hand, it has only been 6 weeks or so since this was a 1.2 blocker.

Yeah, I know.  The problem is that the Gecko people who would look into this (me or bent) are on the critical path for all sorts of other stuff too :/

Comment 13

[David Flanagan [:djf]]

I've filed bug 966320 for the temporary gaia workaround.  

Resetting the assignee, since I'm not going to be working on the gecko fix in this bug.

Kyle: do you want to take this bug for yourself, or leave it unassigned for now?

Comment 14

[Chris Lee [:clee]]

Marked bug 966320 1.3 blocker and moving this bug to backlog to be assigned into an upcoming sprint in 1.4.

Comment 15

[David Flanagan [:djf]]

The workaround attached above has been landed in the 1.3 tree under bug 966320

Comment 16

[Jason Smith [:jsmith]]

(In reply to Chris Lee [:clee] from comment #14)
> Marked bug 966320 1.3 blocker and moving this bug to backlog to be assigned
> into an upcoming sprint in 1.4.

Actually - we would need to move this to 1.4?, as the fix in bug 966320 only applies to 1.3.

Comment 18

[Jason Smith [:jsmith]]

Kyle - Should we just land the workaround on master & backlog the gecko fix for sometime in the future? Or do you want push for the gecko fix in the 1.4 timeframe?

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'll try to look at this next week.  If I don't make progress we can check in the work around on master.

Comment 20

[Jean Gong :jgong]

PM triage this bug. Should keep in 1.4.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

And now my emulator VM doesn't work anymore.  Sigh.

djf, can you land the workaround on master?

Comment 22

[David Flanagan [:djf]]

Andrew's patch in bug 949941 should make the camera always pass a file-backed blob to MMS and this issue should go away for the camera case.  

The workaround is probably still needed for the case where the user picks an image from the gallery and crops it, however.

The workaround is in the SMS app, not in camera or gallery, and I'd rather have someone from the comms team apply it, since I'm really not familiar with that code base.  It may well be that the patch from bug 966320 will apply cleanly in 1.4, but I'm too sleep deprived right now from camera work to risk touching an app that I don't know well.

Julien is away, so setting needinfo for David S to find someone to take this.

Comment 23

[David Scravaglieri [:scravag]]

Julien, could you take a look at David's proposition please ?

Comment 24

[Julien Wajsberg [:julienw]]

Will do

Comment 25

[Julien Wajsberg [:julienw]]

David, I don't follow everything, but would the patch in bug 975599 work around this gecko issue too?

Comment 26

[David Flanagan [:djf]]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #25)
> David, I don't follow everything, but would the patch in bug 975599 work
> around this gecko issue too?

Yes, I think that would work.  Jason: could you have someone from QA test that?  Apply the patch pending review in 975499 and then try to reproduce this bug?  Even though the title of this bug says it is about camera, we think the camera case is already fixed. What we're concerned about here is using the SMS app to pick an image from gallery, and cropping that image as far as possible (so that the SMS app uses it unmodified without resizing it itself).

Comment 27

[Jason Smith [:jsmith]]

Okay - adding qawanted to test what's stated in comment 26.

Comment 28

[Matthew Vaughan]

(In reply to Jason Smith [:jsmith] from comment #27)
> Okay - adding qawanted to test what's stated in comment 26.

After applying the patch from bug 975599, this issue does *not* reproduce for me on the 03/27/14 1.4 build. I was able to view the maximum cropped images that I attached to a MMS from the Gallery.

- 1.4 build with patch applied -
Device: Buri v1.4 MOZ RIL
BuildID: 20140327074806
Gaia: 57ea6d3389d8ed27d2c65614234017e1227559b9
Gecko: 69e896713b11
Version: 30.0a2
Firmware Version: v1.2-device.cfg

Comment 29

[Jason Smith [:jsmith]]

(In reply to Matthew Vaughan from comment #28)
> (In reply to Jason Smith [:jsmith] from comment #27)
> > Okay - adding qawanted to test what's stated in comment 26.
> 
> After applying the patch from bug 975599, this issue does *not* reproduce
> for me on the 03/27/14 1.4 build. I was able to view the maximum cropped
> images that I attached to a MMS from the Gallery.
> 
> - 1.4 build with patch applied -
> Device: Buri v1.4 MOZ RIL
> BuildID: 20140327074806
> Gaia: 57ea6d3389d8ed27d2c65614234017e1227559b9
> Gecko: 69e896713b11
> Version: 30.0a2
> Firmware Version: v1.2-device.cfg

You didn't follow the directions here. The testing request was to test to see if *this* bug was still reproducible with that patch. It isn't talking about testing the other bug's STR.

Comment 30

[Matthew Vaughan]

(In reply to Jason Smith [:jsmith] from comment #29)
> (In reply to Matthew Vaughan from comment #28)
> > (In reply to Jason Smith [:jsmith] from comment #27)
> > > Okay - adding qawanted to test what's stated in comment 26.
> > 
> > After applying the patch from bug 975599, this issue does *not* reproduce
> > for me on the 03/27/14 1.4 build. I was able to view the maximum cropped
> > images that I attached to a MMS from the Gallery.
> > 
> > - 1.4 build with patch applied -
> > Device: Buri v1.4 MOZ RIL
> > BuildID: 20140327074806
> > Gaia: 57ea6d3389d8ed27d2c65614234017e1227559b9
> > Gecko: 69e896713b11
> > Version: 30.0a2
> > Firmware Version: v1.2-device.cfg
> 
> You didn't follow the directions here. The testing request was to test to
> see if *this* bug was still reproducible with that patch. It isn't talking
> about testing the other bug's STR.

Jason, could you please clarify that *this* bug refers to being unable to view specific small pictures (200-300KB in size) from MMS attachment taken from the camera?

The steps I followed to test this issue are:
1) Launch the Message app > open a new message
2) Select the paperclip icon to attach a file
3) Tap 'Camera' > take a photo > tap 'Select'

**All the photos I took were 30-80KB in size**

4) Tap the attached image > tap 'View'


With these STR, I was not able to reproduce this issue with and without the patch from bug 975599. This looks to be because the photo sizes don't get any larger than 80KB.

Is this the correct STR I should be using, or is there another set of steps that I need to run through to test this issue?

Comment 31

[Jason Smith [:jsmith]]

Don't think so - here's the potential STR below. Can we reverify these sets of STR reproduce the bug? I don't think the testing above matches the original way to reproduce this.

STR:
1. Launch SMS
2. Add a new sms
3. Click add attachment -> Camera
4. Take a picture with size 200K~300K.
   This is very important! You can try to take a picture of a while paper, the size will be 200K~300K.
5. Click "Select"
6. In the new message page, tap on the picture and select "View"
   -> [issue] Unable to view this picture. It always return to a untitled page.

Repro Steps:
1) Updated Buri Build ID: 20131204115608
2) Open Messages app
3) Tap compose icon to begin new message
4) Enter recipient information
5) Select paperclip icon to attach media
6) Press gallery
7) Tap existing image
8) Drag crop selection so that only rightmost portion of image is selected and press "Done"
9) Send message
10) Tap on thumbnail of picture just sent

Actual:
Cropped images sent by MMS cannot be opened by sender.

Expected:
Tapping on sent MMS image opens full image.

Comment 32

[Matthew Vaughan]

(In reply to Jason Smith [:jsmith] from comment #31)
> Don't think so - here's the potential STR below. Can we reverify these sets
> of STR reproduce the bug? I don't think the testing above matches the
> original way to reproduce this.
> 
> STR:
> 1. Launch SMS
> 2. Add a new sms
> 3. Click add attachment -> Camera
> 4. Take a picture with size 200K~300K.
>    This is very important! You can try to take a picture of a while paper,
> the size will be 200K~300K.
> 5. Click "Select"
> 6. In the new message page, tap on the picture and select "View"
>    -> [issue] Unable to view this picture. It always return to a untitled
> page.

Using these STR, the issue does *not* reproduce on the 03/31/14 1.4 build with the aforementioned patch applied. Each picture I took would range between about 20-80KB, even after taking a picture of a white sheet of paper. No matter what I tried, the images would not come close to the needed 200-300KB to reproduce the bug... i.e. the images appear to be re-sized when they are attached.
 
> Repro Steps:
> 1) Updated Buri Build ID: 20131204115608
> 2) Open Messages app
> 3) Tap compose icon to begin new message
> 4) Enter recipient information
> 5) Select paperclip icon to attach media
> 6) Press gallery
> 7) Tap existing image
> 8) Drag crop selection so that only rightmost portion of image is selected
> and press "Done"
> 9) Send message
> 10) Tap on thumbnail of picture just sent
> 
> Actual:
> Cropped images sent by MMS cannot be opened by sender.
> 
> Expected:
> Tapping on sent MMS image opens full image.

Using these STR, the issue does *not* reproduce on the 03/31/14 1.4 build with the aforementioned patch applied. The images could be viewed before and after they were sent. Additionally, I could view the same images when they were sent back to the test device.


- 1.4 build with patch -
Device: Buri v1.4 MOZ RIL
BuildID: 20140331000202
Gaia: 57ea6d3389d8ed27d2c65614234017e1227559b9
Gecko: 3aaca223b673
Version: 30.0a2
Firmware Version: v1.2-device.cfg

Comment 33

[Jason Smith [:jsmith]]

Marking as fixed then by the patch then, since it's landed.

Comment 34

[Julien Wajsberg [:julienw]]

Jason, there is still a Gecko issue that was worked around by the Gallery patch, we should keep this bug open or file a new one.

Comment 35

[Jason Smith [:jsmith]]

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #34)
> Jason, there is still a Gecko issue that was worked around by the Gallery
> patch, we should keep this bug open or file a new one.

Let's file a new bug about the underlying gecko issue.

Comment 36

[Julien Wajsberg [:julienw]]

I filed bug 990123