1181371 - [Gallery] In Edit Mode, toggling 'Auto-Enhance' then tapping the 'X' in top left will do nothing

Status: VERIFIED FIXED

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

Description

[Oliver Nelson [:oliverthor]]

Description:
If the user is editing an image in their gallery and they tap the 'Auto-Enhance' wand button (far right icon), they may observe that attempting to exit afterwards by tapping the 'X', without tapping other 'Edit' buttons, will have no effect. Tapping the 'X' a second time, from this point, will then close the edit view.

PreReq:
* have an image in gallery
Repro Steps:
1) Update a Aries to 20150707033152
2) Open gallery
3) Tap on image
4) Tap 'Edit' button
5) Tap 'Auto-Enhance' wand button (far-right).
6) Tap 'X'

Actual:
Edit view is not closed, tapping 'X' again will close

Expected:
Edit view is closed


Environmental Variables:
-----------------------------

Device: Aries 2.5
Build ID: 20150707033152
Gaia: e6506d68e01489b57616f8b74e8773f938ce62b3
Gecko: e271ef4c49ae
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Flame 2.5
BuildID: 20150707010204
Gaia: e6506d68e01489b57616f8b74e8773f938ce62b3
Gecko: e7e69cc8c07b
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 42.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Flame 2.2
BuildID: 20150707002504
Gaia: ea11f422b687a982f0a961c9aea7858066561707
Gecko: 52d19a805d35
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
********************

Issue DOES NOT REPRODUCE on 2.1 for flame devices
Reslts: Tapping 'Auto-Enhance' then tapping 'X' will close the edit view

Device: Flame 2.1
BuildID: 20150707001206
Gaia: d13826b20b4a45e3f5cd4b25a30a737d8be7f1b9
Gecko: 37de99064c33
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-----------------------------

Repro frequency: 5/5
See attached: 
video: will upload link tomorrow, youtube is disagreeing with me
logcat

Comment 1

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150708_1522.txt

Comment 2

[Oliver Nelson [:oliverthor]]

video posted to url:
https://youtu.be/qw0-47OAFOo

Comment 3

[Peter Bylenga [:PBylenga]]

Small regression, NI on component owner for nomination decision and assignment.

Comment 4

[No-Jun Park [:njpark]]

[Blocking Requested - why for this release]:

I think it's worth blocking, since it's quite a common use case and obvious regression.

Comment 5

[Punam Dahiya [:pdahiya]]

Taking bug to investigate and submit fix

Comment 6

[Punam Dahiya [:pdahiya]]

Investigated and this issue is regression caused by refactoring image editing as two steps process (Bug 1087494), Auto enhance edit tool is single step and stays on main edit screen. 

For Auto enhance, Cancel event handler on first cancel tap has currentEditTool set to 'enhance' and that's why not exiting main edit screen without saving edited image.

https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/js/ImageEditor.js#L553

I will submit patch to update cancel event handler for enhance to exit from main edit screen on first tap.

Comment 7

[GitHub Autolander Bot]

Attachment: [gaia] punamdahiya:Bug1181371 > mozilla-b2g:master

Comment 8

[Punam Dahiya [:pdahiya]]

Comment on attachment 8636692 [details] [review]
[gaia] punamdahiya:Bug1181371 > mozilla-b2g:master

Hi David

Attaching patch that on Cancel checks for enhance as currentEditTool and exit main edit screen. Please review. Thanks!

Comment 9

[Hema Koka [:hema]]

Blocking Reason: regression

Comment 10

[David Flanagan [:djf]]

Comment on attachment 8636692 [details] [review]
[gaia] punamdahiya:Bug1181371 > mozilla-b2g:master

I didn't test, but this looks good to me.

Comment 11

[Hubert Figuiere [:hub]]

Should we have automated testing for this? Sounds like a good candidate.

Comment 12

[No-Jun Park [:njpark]]

(In reply to Hubert Figuiere [:hub] from comment #11)
> Should we have automated testing for this? Sounds like a good candidate.

I think so too, I'll put it in my todo list, and hopefully get on it this quarter, unless someone else can work on it beforehand.

Comment 13

[Punam Dahiya [:pdahiya]]

Thanks Hub for feedback. I will update patch with test relevant to this fix. 

No-Jun, It will be very helpful if we can have similar test for all edit tool flows which can be landed as a separate bug. Thank you so much for your help with the tests!

Comment 14

[Punam Dahiya [:pdahiya]]

Comment on attachment 8636692 [details] [review]
[gaia] punamdahiya:Bug1181371 > mozilla-b2g:master

Hi No-Jun

Asking for your review of tests in updated patch as you are familiar with gallery marionette tests. Thanks!

Comment 15

[No-Jun Park [:njpark]]

Comment on attachment 8636692 [details] [review]
[gaia] punamdahiya:Bug1181371 > mozilla-b2g:master

Ran it locally and LGTM.

Comment 16

[Punam Dahiya [:pdahiya]]

Thanks No-Jun and David for review. Patch landed on master

https://github.com/mozilla-b2g/gaia/commit/b34683f9a46dcd9d47ff51174b17917510eda05b

Comment 17

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Flame and Aries. Following STR, tapping on X correctly closes the edit mode window.

Device: Aries 2.5
BuildID: 20150804120650
Gaia: 67c38af8347f93ddc005a53f427d651b744b55c1
Gecko: 5cf4d2f7f2f2
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Flame 2.5
BuildID: 20150804030213
Gaia: caba8b26c52d3c771e9ea6fe288acdaf74c7707e
Gecko: 5b54831761b1
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0