1183570 - [Browser]When user taps 'Share' and 'Cancel', share activity picker should animate out from the bottom.

Status: VERIFIED FIXED

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

Description

[helen（leave from Mozilla）]

Attachment: Flame_master.3gp

[1.Description]:
According to comment 19 in bug 1141496, this bug filed.
[Flame master] & [Nexus 5 master][Browser]When user taps 'Share' at the website page in browser app, share activity picker appears from the bottom but it will disppear immediately without animation, and after you tap 'Cancel' it will disppear immediately without animation too.

Occur Time: 08:18
Attachment: Flame_master.3gp & logcat0818.txt

[2.Testing Steps]: 
1.Launch browser, tap URL bar and enter any website. 
2.Tap menu icon at top right corner and pick 'Share'.
3.Tap 'Cancel'.

[3.Expected Result]: 
2&3.When user taps 'Share' and 'Cancel', share activity picker should animate out from the bottom.

[4.Actual Result]: 
2&3.When tap 'Share' and 'Cancel', share activity picker doesn't animate out from the bottom.

[5.Reproduction build]: 
Device: Flame 2.2 (unaffected)
Build ID               20150713002503
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150713.041147
Firmware Date          Mon Jul 13 04:11:59 EDT 2015
Bootloader             L1TC000118D0

Device: Flame master (affected)
Build ID               20150713010204
Gaia Revision          e4b63559eba364892867eb381c3002d6518e5d6a
Gaia Date              2015-07-10 14:29:23
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eab21ec484bb
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150713.043935
Firmware Date          Mon Jul 13 04:39:47 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 2.2 (unaffected)
Build ID               20150713002503
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150713.040339
Firmware Date          Mon Jul 13 04:04:00 EDT 2015
Bootloader             HHZ12f

Device: Nexus5 master (affected)
Build ID               20150713010204
Gaia Revision          e4b63559eba364892867eb381c3002d6518e5d6a
Gaia Date              2015-07-10 14:29:23
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eab21ec484bb
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150713.043623
Firmware Date          Mon Jul 13 04:36:39 EDT 2015
Bootloader             HHZ12f


[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

Comment 1

[helen（leave from Mozilla）]

Attachment: logcat0818.txt

Comment 2

[Chris Lord [:cwiiis]]

To clarify, both dialogs animate out from the bottom, but they don't animate away back to the bottom once dismissed.

Comment 3

[John Thomas [:Johnt]]

b2g Inbound Regression Window

First Working
Environmental Variables:
Device: Flame 2.5
BuildID: 20150503194800
Gaia: 1922c412ba18dc6f5ba025bb9cf8fa1f68a9c0dc
Gecko: 189e5afff783
Version: 40.0a1 (2.5) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0



First Broken
Environmental Variables:
Device: Flame 2.5
BuildID: 20150503211700
Gaia: bb65538727c74f971c89c9d50aefa59c5b366bf0
Gecko: 8832ff5d030c
Version: 40.0a1 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0




Last Working gaia / First Broken gecko - This issue DOES occur.
Gaia: 1922c412ba18dc6f5ba025bb9cf8fa1f68a9c0dc
Gecko: 8832ff5d030c


Last Working gecko / First Broken gaia - This issue does occur.
Gecko: 189e5afff783
Gaia: bb65538727c74f971c89c9d50aefa59c5b366bf0


b2g Inbound Pushlog:
https://github.com/mozilla-b2g/gaia/compare/1922c412ba18dc6f5ba025bb9cf8fa1f68a9c0dc...bb65538727c74f971c89c9d50aefa59c5b366bf0

This issue most likely occurs due to Bug 1154642 OR Bug 1158238

Comment 4

[KTucker [:KTucker][Inactive 3/4/2016]]

John, can you take a look at this please? We think that the landing for bug 1154642 might be the cause here but we are unsure.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Let me check it. Thanks.

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

Well, I think it is created by my patch after viewing the video.

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

Let me explain more. The original design is wrong even without my patch.

All animations should be visible while running. At hiding case, we set visibility to hidden directly by remove visible class. This behavior makes the UI invisible but animation is still running. In contrast, we set visibility to visible while showing. The animation runs correctly with the visible UI. So, I will create a patch to have hiding state to make UI visible at this state. After animation ended, it goes back to hidden state.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master

Comment 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to John Thomas [:Johnt] from comment #3)
> b2g Inbound Regression Window
> 
> Last Working gaia / First Broken gecko - This issue DOES occur.
> Gaia: 1922c412ba18dc6f5ba025bb9cf8fa1f68a9c0dc
> Gecko: 8832ff5d030c
> 
> 
> Last Working gecko / First Broken gaia - This issue does occur.
> Gecko: 189e5afff783
> Gaia: bb65538727c74f971c89c9d50aefa59c5b366bf0
> 

Hi John Thomas,

What does this mean? Which one:
Last Working Gaia First Broken Gecko: Issue DOES NOT reproduce
First Broken Gaia Last Working Gecko: Issue DOES NOT reproduce

Comment 10

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8633953 [details] [review]
[gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master

Tim,

Please review this patch. This is my fault to produce this regression. The main reason we don't have animation is that we hide the panel and start the animation at the same time. This patch will keep panel visible until panel is closed.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8633953 [details] [review]
[gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master

Sorry for the late reply. This look good unless you have try to transition the CSS visibility property and didn't work. You can save a |hiding| class because of that.

(When you do |transition: visibility|, the element will remain visible during the transition, even if it's transition to a |visibility: hidden| class.)

Comment 12

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #11)
> Comment on attachment 8633953 [details] [review]
> [gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master
> 
> Sorry for the late reply. This look good unless you have try to transition
> the CSS visibility property and didn't work. You can save a |hiding| class
> because of that.
> 
> (When you do |transition: visibility|, the element will remain visible
> during the transition, even if it's transition to a |visibility: hidden|
> class.)

Thanks for the information. We had a regression that focus too early, bug 1158254. So, I remove the transition on visibility. Maybe, that's a wrong decision. The focus function call at transitionend is enough. I will double check that before merging.

Comment 13

[John Hu [:johnhu][:johu][:醬糊小弟]]

Tim,

You are correct. I don't need to remove that transition. Since I put it back, bug 1158254 works correctly. But I will test it with different cases. I had updated the PR. Let check CI's result, too.

Comment 14

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8633953 [details] [review]
[gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master

Tim,

Please see this again. I had changed the patch to leverage `transition: visibility`. This looks elegant.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8633953 [details] [review]
[gaia] john-hu:bug-1183570-context-menu-regression > mozilla-b2g:master

Woot!

Comment 16

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/58a391c6529b47dc40418ae306bce4d4138186b3

tree herder is all green:
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=19143d65d7ef7f01fc9efd204a628b67ea291321

Comment 17

[helen（leave from Mozilla）]

This bug has been verified as pass on latest build of Flame master & Nexus 5 master by the STR in Comment 0. 

Results:
Share activity picker appears from the bottom and it will also exit from bottom after you tap cancel.

Reproduce rate: 0/10

Device: Flame master(Pass)
Build ID               20150721160205
Gaia Revision          84c3bf622e211046d905803b34de5d331761f22d
Gaia Date              2015-07-21 16:28:35
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1875a5584e5f
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150721.194129
Firmware Date          Tue Jul 21 19:41:42 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus5 master (Pass)
Build ID               20150721160205
Gaia Revision          84c3bf622e211046d905803b34de5d331761f22d
Gaia Date              2015-07-21 16:28:35
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/1875a5584e5f
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150721.211608
Firmware Date          Tue Jul 21 21:16:23 EDT 2015
Bootloader             HHZ12f