1059233 - [SMS] User cannot view the cropped image in SMS application and ActivityCanceled error was shown when try to view

Status: RESOLVED DUPLICATE of bug 1063658

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

Description

[sasikala]

1. Title: User cannot view the image which is cropped 
2. Precondition: Store some images in Gallery application
3. Tester's Action: 1.Open Gallery application
                    2.Select any image -> crop -> select Done 
                    3.Select the image which is attached in SMS app
                    4.Click on View button

4. Actual Symptom (ENG.) : When user click on attached image -> image is not shown and we are receiving the ActivityCanceled error when trying to open an image
5. Expected: User should be able to view all the images at any time.

Comment 1

[sasikala]

When trying to open the attached cropped image -> it's giving ActivityCanceled error

08-27 17:16:05.339: E/GeckoConsole(6417): Content JS ERROR at app://sms.gaiamobile.org/js/attachment.js:256 in Attachment.prototype.view/activity.onerror: error with open activity ActivityCanceled

Comment 2

[jongsoo.oh]

That is caused message error
###!!! [Parent][DispatchAsyncMessage] Error: Value error: message was deserialized, but contained an illegal value

Comment 3

[Wayne Chang [:wchang]]

Steve,

Have you got this covered? I vaguely remember seeing something similar before but I maybe wrong...

Comment 4

[Oleg Zasypkin [:azasypkin]]

Attachment: gallery.log

When I see this issue, "storage is null" exception is thrown in gallery app. After that I can't reopen gallery - "No memory card" error is displayed until I reboot device. Attaching logcat.

Comment 5

[Oleg Zasypkin [:azasypkin]]

Redirecting to gallery to confirm...

Comment 6

[Oleg Zasypkin [:azasypkin]]

Just a note, looks like it happens only when we pass "allowSave=true" to MozActivity.

Comment 7

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Steve, Oleg, so are you going to take this one, or should we wait for Gallery guy's comments?

Comment 8

[Steve Chung [:steveck]]

Hi Vince, I'm afraid that there is no much thing message app could right now.

Although David suggested us that message could make a local copy instead of opening the blob returning from gallery directly long time ago, the problem is still reproducible even we send the message and open the image in thread immediately. Image should be saved into message DB at this moment but still unable to open at the very beginning(the reproduce rate is low and it could be opened after a while). I suspect the regression in gecko side, but maybe gallery devs could know the changes about this part.

Comment 9

[jongsoo.oh]

The sms request the image data to attachment, the gallery give to the blob data.
If the blob size is smaller than Settings.mmsAttachSizeLimitation, it does not resize 

v2.0 sms app - composer.js
298    if (Compose.attachmentSize <= Settings.mmsAttachSizeLimitation) {
299      onContentChanged();
300      return;
301    }
In this case this issue reproduce.
If the image resized in sms app using  Utils.getResizedImgBlob, this issue is not reproduced.

Comment 10

[jongsoo.oh]

I checked Flame device
OS: 2.0.0.0-prerelease
Build ID: 20140829075332

Action
1. Open sms app - new message
2. select attach button - select Gallery - select image(the image size is 284K)
3. the Gallery give the 86K blob - the thumbnail is shown in sms app
4. select thumbnail - select view - can not view the image

Comment 11

[Steve Chung [:steveck]]

Attachment: Link to github

Hey Oleg, since this issue might also block our patch in bug 983172, I still make a quick fix by making a local copy. Do you think we should take this workaround(?) before any reply from gallery?

Comment 12

[Steve Chung [:steveck]]

Comment on attachment 8487728 [details] [review]
Link to github

Hi Oleg, I updated the patch as we discussed on IRC ;)

Wayne, you are right that we did fix this issue before, but only on specific version(v1.2/v1.3t). There is one bug created for gecko solution(bug 990123) but we are not sure when it would be addressed. So Oleg and I decided to land the workaround on master since other patch will need this fixing to avoid app crash.

Comment 13

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8487728 [details] [review]
Link to github

Works great, thanks Steve! Just few nits on Github.

I still see some issues with previewing thumbnails\attaching files from Gallery and Video (no sd card and etc.), but as per logcat it looks more like bug 1063912 or bug 1063877 that currently on track.

Comment 14

[Steve Chung [:steveck]]

(In reply to Oleg Zasypkin [:azasypkin] from comment #13)
> Comment on attachment 8487728 [details] [review]
> Link to github
> 
> Works great, thanks Steve! Just few nits on Github.
> 
> I still see some issues with previewing thumbnails\attaching files from
> Gallery and Video (no sd card and etc.), but as per logcat it looks more
> like bug 1063912 or bug 1063877 that currently on track.

Hmm, will track bug 1063877 then. Thanks!

In master: d6f841f5bef301f5b19b03f5bf149b4c65b97f5e

Comment 15

[Julien Wajsberg [:julienw]]

Steve, Oleg, how do you feel about creating an integration test for this?

Comment 16

[Julien Wajsberg [:julienw]]

We need to decide whether we need this in any branch.

From IRC:

<djf> julienw: the gallery workaround was introduced on March 27th in bug 975599. It was uplifted to 1.4 and 1.3T.  It was then removed by bug 1006919 on May 7th. It looks like that patch that removed it was also uplifted to 1.4, so the only release that still has the workaround is 1.3T.  Our justification for removing the workaround was that bug 982779 had been fixed.  Hope this helps.

That means that this bug should not happen and there may be an issue in Gecko. I'll try on v2.0 tomorrow.

Comment 17

[Julien Wajsberg [:julienw]]

QA: can you do branch checks and regression windows on this bug? It's been fixed but I want to know if we need to uplift, and maybe we landed a wrong fix here.

Please also test v1.3 if possible. Thanks!

Comment 18

[Julien Wajsberg [:julienw]]

Hey Joshua, is there a chance you can make the QA requests here happen? Even if the bug is marked "resolved fixed" it's actually quite urgent as we need it for bug 1063658 and we'd like to evaluate whether a fix is needed in v2.0. Thanks !

Comment 19

[Joshua Mitchell (Inactive)]

Can do!

Comment 20

[Andrew Sutherland [:asuth] (he/him)]

So, based on my understanding of the underlying memory-backed problem that is bug 1063658, I think this fix is still depending on the SMS app winning a race.  The only mitigation that will definitely work 100% is making the gallery app/whatever pass you a DeviceStorage-file-backed or IndexedDB-file-backed Blob again.

More specifically, when email gets hit by this bug, it's possible for the originating app (gallery) to have already died by the time the email app actually gets the onsuccess notification.  I specifically reproduced that on a debug build on a trunk flame, but the scenario still holds on a device that's running a normal build but with some resource contention/etc.

Comment 21

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue occured on 2.2 KK Flame, 2.1 KK Flame, 2.0 KK Flame, and the 2.0 Flame on the v123 JB base.

Environmental Variables:
Device: Flame 2.2
BuildID: 20140904171737
Gaia: de59e0c3614dd0061881fe284e9f2d74fa0d1d5d
Gecko: 8703c1895505
Version: 35.0a1 (2.2) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables:
Device: Flame 2.1
BuildID: 20140904062538
Gaia: a47ecb6368c015dd72148acde26413fd90ba3136
Gecko: ffb144a500a4
Version: 34.0a2 (2.1) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
BuildID: 20140904114640
Gaia: 13978cf2230652274969536322378d448fd142a4
Gecko: 2537ab191112
Version: 32.0 (2.0) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20140829061900
Gaia: bee44b6fa2f19db606359d6756638f63097cf4dc
Gecko: ba9759c7b3b3
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

This issue did not occur on the v165 1.4 Flame KK base or the latest 2.2 KK Flame.

Environmental Variables:
Device: Flame 2.2
BuildID: 20140917212258
Gaia: d37950eb09e28aa18d0e01df9ff90574bd4337e0
Gecko: 426497473505
Version: 35.0a1 (2.2) 
Firmware Version: 
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Comment 22

[Joshua Mitchell (Inactive)]

(In reply to Julien Wajsberg [:julienw] from comment #17)
> QA: can you do branch checks and regression windows on this bug? It's been
> fixed but I want to know if we need to uplift, and maybe we landed a wrong
> fix here.
> 
> Please also test v1.3 if possible. Thanks!

Jayme - can you also check v1.3 here (JB base only)

Comment 23

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue does not occur on the v123 1.3 JB base.

Comment 24

[Joshua Mitchell (Inactive)]

ok - so this IS a regression - let's get the window now.

Comment 25

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Bug 1006919 is the workaround removal Julien was talking about and is in the pushlog.

B2g-inbound Regression Window

Last working 
Environmental Variables:
Device: Flame 2.0
BuildID: 20140522143004
Gaia: b61129780e085636d09406f2a46e922d0f8b9757
Gecko: d2d52126161d
Version: 32.0a1 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


First Broken 
Environmental Variables:
Device: Flame 2.0
BuildID: 20140522173006
Gaia: 44fed8d50581dfc3b61386c77531aeba7f6c5f38
Gecko: ccdd8ab3ec67
Version: 32.0a1 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Last working gaia / First broken gecko - Issue does NOT occur
Gaia: b61129780e085636d09406f2a46e922d0f8b9757
Gecko: ccdd8ab3ec67

First broken gaia / Last working gecko -Issue DOES occur
Gaia: 44fed8d50581dfc3b61386c77531aeba7f6c5f38
Gecko: d2d52126161d

Gaia Pushlog:  https://github.com/mozilla-b2g/gaia/compare/b61129780e085636d09406f2a46e922d0f8b9757...44fed8d50581dfc3b61386c77531aeba7f6c5f38

Comment 26

[Joshua Mitchell (Inactive)]

Julien - NI to you to grab your attention - Bug 1006919 is 1 of the 2 bugs in the pushlog

Comment 28

[Julien Wajsberg [:julienw]]

Thank you so much Joshua and Jayme, this is really useful. At least this shows us this is about the issue we had in mind :)

What I don't get though is that bug 1006919 is in v1.4. I think Andrew is right here and maybe the STR is not good enough. I'll do more tests by myself so keeping the NI.

Comment 29

[Julien Wajsberg [:julienw]]

Jayme, can you confirm the STR you used is this?

1.Open SMS app, start a new message
2.Start a "pick" activity, choose "Gallery"
3.Select any image -> crop -> select Done 
4.Select the image which is attached in SMS app
5.Click on View button

The first step was missing in comment 0.

Comment 30

[Julien Wajsberg [:julienw]]

When I try to flash a build from around the regression window, I always get a crash at step 3, after "select any image" (Flame or Hamachi, tried 2 different builds). Do you get this as well?

Comment 31

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Julien,
I did do follow those steps, though to reproduce I had to crop an image with the gallery 3 times first so it was small enough after cropping again in step 3.  I did not see a crash when cropping the image.

Comment 32

[Julien Wajsberg [:julienw]]

Can I ask you where you get the builds?

Comment 33

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Julien,
We have a repository of Tinderbox builds for Flame Engineering that we've collected since mid April since they are cleared out after three weeks.

Comment 34

[Julien Wajsberg [:julienw]]

Is it possible to share it with us?

Comment 35

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Peter or Joshua might know.

Comment 36

[Peter Bylenga [:PBylenga]]

Hi Julien, I sent an email offline about this.

Comment 37

[Julien Wajsberg [:julienw]]

reverted in a70e9612e17ecb8b13e95d523a07034c758e9d0a