1111850 - [MMS][Gallery] Missing error message when attaching an image to a MMS while USB storage is enabled.

Status: VERIFIED FIXED

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

Description

[Seth Miko [:SMiko]]

Attachment: Logcat

Description: The user is shown a blank gallery instead of the error message when attaching an image to a MMS while USB storage is enabled.

Repro Steps:
1: Update a Flame to 20141215040201
2: Plug the device into a computer and enable USB storage.
3: Open Messaging > Create new message
4: Tap on the paper clip icon and select Gallery.

Actual: The gallery launches but no images are displayed. 

Expected: The error message "Memory card in use, Unplug the phone to use this app" is displayed.

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141215040201
Gaia: e2a3e606675c346b6e6f35351a458040be599b09
Gecko: f14dcd1c8c0b
Gonk: 263b5f41f7733c5577fb101eb4dc8ac5c11cfa8d
Version: 37.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Repro frequency: 5/5

See attached: logcat

Video clip: https://www.youtube.com/watch?edit=vd&v=P5-gm66hcZ0

Comment 1

[Seth Miko [:SMiko]]

This issue does NOT repro on Flame 2.1 (319mb/KK/full flash)

Actual result:
The error message "Memory card in use, Unplug the phone to use this app" is displayed.

Environmental Variables:
Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
Build ID: 20141215001202
Gaia: 97873dca486abf4162a3345e71b375806937bdec
Gecko: ec87f4f41d3d
Gonk: 263b5f41f7733c5577fb101eb4dc8ac5c11cfa8d
Version: 34.0 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

[Oleg Zasypkin [:azasypkin]]

Sounds like gallery issue, Camera and Video show that warning.

Comment 3

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

It seems this is the regression:

https://github.com/mozilla-b2g/gaia/commit/47d2c31c562939af368f028b4e5371aa65837729#diff-508f3a8a4cb447602009081be5eb9236L1046

The function was removed but not in the whole app. I'm going to remove the regressionwindow-wanted tag to avoid wasting time and resources. Feel free to request the regression window if you want.

Thanks

Comment 4

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20141105120748
Gaia: 7918024c737c4570cacd784f267e28737ae05dea
Gecko: b6cd2dd85b26
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First broken
BuildID: 20141106040023
Gaia: 068b9711277b06c7d633517f9e1fcb5624bb39b3
Gecko: a074c0112919
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Working Gaia / Broken Gecko = No repro
Gaia: 7918024c737c4570cacd784f267e28737ae05dea
Gecko: a074c0112919
Broken Gaia / Working Gecko = Repro
Gaia: 068b9711277b06c7d633517f9e1fcb5624bb39b3
Gecko: b6cd2dd85b26
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/7918024c737c4570cacd784f267e28737ae05dea...068b9711277b06c7d633517f9e1fcb5624bb39b3


B2G Inbound
Last working
BuildID: 20141105103947
Gaia: a7904af8e602d3f54c31aeb44a4a9773f99265f6
Gecko: 1139ec0f1ac3
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First broken
BuildID: 20141105105744
Gaia: bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e
Gecko: 22b67157c1d7
Platform Version: 36.0a1
Firmware Version: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Working Gaia / Broken Gecko = No repro
Gaia: a7904af8e602d3f54c31aeb44a4a9773f99265f6
Gecko: 22b67157c1d7
Broken Gaia / Working Gecko = Repro
Gaia: bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e
Gecko: 1139ec0f1ac3
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/a7904af8e602d3f54c31aeb44a4a9773f99265f6...bfafc5a2ca03480c0c33abaacd35ee0d0ade3b8e

Comment 5

[Joshua Mitchell (Inactive)]

Tester had already started on the window so I just had him finish it.

Window confirms comment 3 - regression caused by patch to bug 1064600

Bug is already assigned and patch attached so taking no further action at this time

Comment 6

[Punam Dahiya [:pdahiya]]

(In reply to Manuel Casas Barrado [:mancas] from comment #3)
> Created attachment 8537152 [details] [review]
> Proposed patch
> 
> It seems this is the regression:
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 47d2c31c562939af368f028b4e5371aa65837729#diff-
> 508f3a8a4cb447602009081be5eb9236L1046
> 
> The function was removed but not in the whole app. I'm going to remove the
> regressionwindow-wanted tag to avoid wasting time and resources. Feel free
> to request the regression window if you want.
> 
> Thanks

Thanks Manuel for helping debug the issue.  cleanupCrop is redundant function and the main reason behind not showing up message 'Memory card in use' for pick activity when USB is mounted.

However we do need to reset crop editor when switching back to pick view. Here are steps to replicate crop overlay seen from previous cropped image

1. Open messaging app > create new message
2. Attach image from gallery 
3. Select an image and crop it by dragging crop handles
4. Enable USB Storage in settings app , go back to messaging app, shows message 'Memory card in use, Unplug the phone to use this app'
5. Disable USB storage and unplug phone.
6. Return to messaging app, shows select screen (pick view) of gallery
7. Select an image , crop screen shows the previous cropped image

Expected: Previous cropped image should not be shown

I will recommend handling this case with this bug by using Pick.restart() at L270 (See github) which resets cropEditor and sets view to Pick Mode. Thanks

Comment 7

[Punam Dahiya [:pdahiya]]

Comment on attachment 8537152 [details] [review]
Proposed patch

r- for the reason provided in #comment 6. Including Justin who has written Pick module, for feedback and help review the patch with the approach suggested in #comment 6. Thanks

Comment 8

[Hema Koka [:hema]]

Blocking Reason: Regression and cannot access gallery

Comment 9

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8537152 [details] [review]
Proposed patch

I took into account your comments, please check the new approach

Thanks

Comment 10

[Punam Dahiya [:pdahiya]]

Comment on attachment 8537152 [details] [review]
Proposed patch

Patch looks good and has r+. Thanks!

Comment 12

[Punam Dahiya [:pdahiya]]

Hi Manuel

Please let me know if you need help to get this patch landed on master. Thanks!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/e35af4d9857d53bb05bba81cee0b4a058b42c085

Comment 14

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on latest nightly build of Flame v2.2&Master by the STR in Comment 0.

Actual results: The error message "Memory card in use. Unplug the phone to use this app" shows as expected.
See attachment: verified_Flame_v2.2.3gp
Reproduce rate: 0/10


Device: Flame v2.2 (Verified) 
Build ID               20150709162504
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e002005dc994
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150709.202124
Firmware Date          Thu Jul  9 20:21:35 EDT 2015
Bootloader             L1TC000118D0

Device: Flame master (Verified)
Build ID               20150709160207
Gaia Revision          bdddfe1ebb796e2bc1c048d5c4e0f97f3d06f98b
Gaia Date              2015-07-09 11:58:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/adfdc7f29ba7
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150709.193215
Firmware Date          Thu Jul  9 19:32:27 EDT 2015
Bootloader             L1TC000118D0

----------------------------------------------------------------------------------------------------
Note: 
When we execute the STR of comment 6 on latest Flame v2.2&master, previous cropped image is not shown as expected.

Comment 15

[Shally Li（leave from Mozilla）]

Attachment: verified_Flame_v2.2.3gp