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

RESOLVED FIXED

Status

Firefox OS
Gaia::Gallery
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jsmith, Assigned: khuey)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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+.
(Reporter)

Updated

4 years ago
Blocks: 944276
blocking-b2g: --- → 1.3+
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.
Flags: needinfo?(khuey)
Duplicate of this bug: 957931
Hey Kyle, do you think you'll be able to have a look soon? Or should we apply bug 944276 in 1.3 too?
PM retriaged this and we agree with the blocking status.

Comment 5

4 years ago
(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?
Keywords: qawanted
(Reporter)

Comment 6

4 years ago
(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.
(Reporter)

Updated

4 years ago
Keywords: qawanted

Comment 7

4 years ago
Kyle: Would be able to take this on? We are shooting to get 1.3+ bugs fixed as soon as possible (by 1/31)
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.
Flags: needinfo?(khuey) → needinfo?(dflanagan)
Created attachment 8367676 [details] [review]
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.
Attachment #8367676 - Flags: review?(felash)
Flags: needinfo?(dflanagan)
I'd rather fire a new 1.3+ bug for this and make this one 1.4+.
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.
Attachment #8367676 - Flags: review?(felash) → review+
> 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 :/
Assignee: nobody → dflanagan
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?
Assignee: dflanagan → nobody
Flags: needinfo?(khuey)

Comment 14

4 years ago
Marked bug 966320 1.3 blocker and moving this bug to backlog to be assigned into an upcoming sprint in 1.4.
blocking-b2g: 1.3+ → backlog
Assignee: nobody → khuey
Flags: needinfo?(khuey)
The workaround attached above has been landed in the 1.3 tree under bug 966320
(Reporter)

Comment 16

4 years ago
(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.
blocking-b2g: backlog → 1.4?
Duplicate of this bug: 969216
(Reporter)

Updated

4 years ago
blocking-b2g: 1.4? → 1.4+
(Reporter)

Comment 18

4 years ago
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?
Flags: needinfo?(khuey)
I'll try to look at this next week.  If I don't make progress we can check in the work around on master.
Flags: needinfo?(khuey)

Comment 20

4 years ago
PM triage this bug. Should keep in 1.4.
And now my emulator VM doesn't work anymore.  Sigh.

djf, can you land the workaround on master?
Flags: needinfo?(dflanagan)
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.
Flags: needinfo?(dflanagan) → needinfo?(dscravaglieri)
Julien, could you take a look at David's proposition please ?
Flags: needinfo?(dscravaglieri)
Will do
Flags: needinfo?(felash)
David, I don't follow everything, but would the patch in bug 975599 work around this gecko issue too?
Flags: needinfo?(felash)
Flags: needinfo?(dflanagan)
(Reporter)

Updated

4 years ago
Keywords: regression
(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).
Flags: needinfo?(dflanagan) → needinfo?(jsmith)
(Reporter)

Comment 27

4 years ago
Okay - adding qawanted to test what's stated in comment 26.
Flags: needinfo?(jsmith)
Keywords: qawanted

Comment 28

4 years ago
(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
Keywords: qawanted
QA Contact: mvaughan
(Reporter)

Comment 29

4 years ago
(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.
Keywords: qawanted

Comment 30

4 years ago
(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?
Keywords: qawanted
(Reporter)

Comment 31

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

Comment 32

4 years ago
(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
Keywords: qawanted
(Reporter)

Comment 33

4 years ago
Marking as fixed then by the patch then, since it's landed.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Updated

4 years ago
status-b2g-v1.4: --- → fixed
status-b2g-v2.0: --- → fixed
Component: DOM → Gaia::Gallery
Product: Core → Firefox OS
Version: Trunk → unspecified
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.
Flags: needinfo?(jsmith)
(Reporter)

Comment 35

4 years ago
(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.
Flags: needinfo?(jsmith)
I filed bug 990123
You need to log in before you can comment on or make changes to this bug.