1112524 - [Flame][Settings]Device will back to previous page and the option will change to your choice, but you doesn't tap ok.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Lance（leave from Mozilla）]

Attachment: logcat_1616.txt

[1.Description]:
[Flame 2.1 & 2.2][Settings]Device will back to previous page and the option will change to your choice, but you tap the sapce between the displaying content and frame to replace ok button.
Found time:16:16
See attachment:1616.mp4 and logcat_1616.txt

[2.Testing Steps]: 
1.Launch Settings.
2.Tap "Date & Time" -> Time Format button.
3.Check another option, then tap the sapce between the displaying content and frame.

Note:This issue exsit in other sub pop up selection page.

[3.Expected Result]: 
3.The page should still display current page.

[4.Actual Result]: 
3.The page will back to previous page, and the option will change to your choice.

[5.Reproduction build]: 
Flame 2.1 version:
Gaia-Rev        79286eafe67707d1330966c1b1413b2d0de595d9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g34_v2_1/rev/222a62b532db
Build-ID        20141216001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141216.034704
FW-Date         Tue Dec 16 03:47:14 EST 2014
Bootloader      L1TC00011880

Flame 2.2 version:
Gaia-Rev        af3d2f89f391c92667e04676fc0ac971e6021bb7
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/a3030140d5df
Build-ID        20141216040205
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141216.075002
FW-Date         Tue Dec 16 07:50:14 EST 2014
Bootloader      L1TC00011880

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

Comment 1

[Lance（leave from Mozilla）]

Attachment: 1616.MP4

Comment 2

[Lance（leave from Mozilla）]

This bug has been verified to fail on Flame 2.1, 2.2.
Issue steps:
1.Launch Settings.
2.Tap "Date & Time" -> Time Format button.
3.Check another option, then tap the sapce between the displaying content and frame.

Note:This issue exsit in other sub pop up selection page.

[3.Expected Result]: 
3.The page should still display current page.

[4.Actual Result]: 
3.The page will back to previous page, and the option will change to your choice.

See attachment: 1549.MP4 and logcat_1549.txt
Reproducing rate: 5/5
Found time:15:49

Flame 2.1 version:
Gaia-Rev        64db236bea9a0510567ab7ced2f2b4688737123c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/273f24a1d1fe
Build-ID        20150111001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.035122
FW-Date         Sun Jan 11 03:51:33 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 version:
Gaia-Rev        f5e481d4caf9ffa561720a6fc9cf521a28bd8439
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/bb8d6034f5f2
Build-ID        20150111010223
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.043244
FW-Date         Sun Jan 11 04:32:55 EST 2015
Bootloader      L1TC000118D0

Comment 3

[Lance（leave from Mozilla）]

Attachment: logcat_1549.txt

Comment 4

[Lance（leave from Mozilla）]

Attachment: 1549.MP4

Comment 5

[Eric Chang [:ericcc] [:echang]]

The spots which dismiss the windows seem to be quite random, but they are all in the middle area between options and okay button.

2.2? for regression.

Comment 6

[Manuel Casas Barrado [:mancas]]

This issue is caused by the edges swipe detector, if we touach and drag from one of the edges the dialog is hidden. What we have to do is to detect that a dialog is opened and disable edge gestures like we did in [1]. Etienne could help here. I can fix it but I will like that Etienne would give me some advices about how we want to fix this issue.

[1] Bug 1097296

Comment 7

[Etienne Segonzac (:etienne)]

(In reply to Manuel Casas Barrado [:mancas] from comment #6)
> This issue is caused by the edges swipe detector, if we touach and drag from
> one of the edges the dialog is hidden. What we have to do is to detect that
> a dialog is opened and disable edge gestures like we did in [1]. Etienne
> could help here. I can fix it but I will like that Etienne would give me
> some advices about how we want to fix this issue.
> 
> [1] Bug 1097296

I'm glad you asked! :)

Disabling the edge gestures should always be the last resort.
Originally alerts/selects/prompts weren't part of the app window element, so we had to cancel them when doing edge gestures.

But since bug 962434 landed we should now be able to safely remove the cancellation [1] and fix this bug.

[1] https://github.com/mozilla-b2g/gaia/blame/793773bb2944b42a85dd160049e605cbd880c4da/apps/system/js/value_selector/value_selector.js#L99-104

Comment 8

[Ted Clancy [:tedders1]]

Attachment: Bug-1112524-fx

This is basically what Étienne said in Comment 7.

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8560201 [details] [review]
Bug-1112524-fx

Almost there, but this is causing a regression:

from the settings, with a value selector opened
- pressing home, then opening the settings back -> the value selector is still here
- but if I open the dialer instead then swipe back to settings -> the value selector isn't displayed and I can't open any value selector anymore...

It might be because in the case of a "swipein" we're not broadcasting _opening, but _opened right away.

Comment 10

[Ted Clancy [:tedders1]]

Hi Étienne,

Are you sure there's a regression? I'm not seeing it.

> pressing home, then opening the settings back -> the value selector is still here

I'm seeing this, but I also see this before my patch is applied (and the behaviour doesn't seem problematic to me). I can make you a video if you want.

> but if I open the dialer instead then swipe back to settings -> the value selector isn't displayed and I can't open any value selector anymore...

I'm not seeing this. Again, I can make you a video if you want.

Comment 11

[Etienne Segonzac (:etienne)]

(In reply to Ted Clancy [:tedders1] from comment #10)
> Hi Étienne,
> 
> Are you sure there's a regression? I'm not seeing it.
> 
> > pressing home, then opening the settings back -> the value selector is still here
> 
> I'm seeing this, but I also see this before my patch is applied (and the
> behaviour doesn't seem problematic to me). I can make you a video if you
> want.

No this is completely alright.

> 
> > but if I open the dialer instead then swipe back to settings -> the value selector isn't displayed and I can't open any value selector anymore...
> 
> I'm not seeing this. Again, I can make you a video if you want.


This is very problematic, you can't access value selectors anymore.
But it's indeed happening on master now, I've file bug 1134710 (patch incoming, looks like it is the missing _opening event).

Comment 12

[Gregor Wagner [:gwagner]]

Can you land this?

Comment 13

[Ted Clancy [:tedders1]]

Green try run: https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=e9ad36c48cbc

Comment 14

[Carsten Book [:Tomcat]]

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

Comment 15

[Sue]

Attachment: verify_fail.MP4

Hi Eric,
Could you help with it? Thanks!
Per comment 14, this issue has fixed on Flame 3.0, but this issue has been verified fail on Flame 3.0
STR:
1.Launch Settings.
2.Tap "Date & Time" -> Time Format button.
3.Check another option, then tap the sapce between the displaying content and frame.
**The page will back to previous page, and the option will change to your choice.
See attachment:verify_fail.MP4
Rate:5/5

Flame 3.0 build:
Build ID               20150226010233
Gaia Revision          7894b929f1b0394f3c997f72a6482bc7813e758d
Gaia Date              2015-02-25 20:50:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/dd6353d61993
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150226.043500
Firmware Date          Thu Feb 26 04:35:10 EST 2015
Bootloader             L1TC000118D0

Comment 16

[Eric Chang [:ericcc] [:echang]]

FWD ni to RD.

Comment 17

[Ted Clancy [:tedders1]]

Yep. This seems to be broken again.

I'll look into it.

Comment 18

[Etienne Segonzac (:etienne)]

Hey Alive, here's a weird scenario:
- open dialer
- open UI Tests app
- open a select in the UI Tests app
- edge swipe to the dialer

=> A select ValueSelector is added to the dialer's AppWindow with the same content than the one from the UI Tests app.

Looks like we get a weird inputmethod-contextchange mozChromeEvent after the app change. Any idea what could cause this?

[opening the select in the ui tests apps]

I/GeckoConsole(27764): Content JS LOG: +++ something from chrome  {"type":"inputmethod-contextchange","inputType":"select-one","value":"","choices":"{\"multiple\":false,\"choices\":[{\"group\":
false,\"inGroup\":false,\"text\":\"Mozilla\",\"disabled\":false,\"selected\":false,\"optionIndex\":0},{\"group\":false,\"inGroup\":false,\"text\":\"B2G\",\"disabled\":false,\"selected\":false,\
"optionIndex\":1},{\"group\":false,\"inGroup\":false,\"text\":\"Gaia\",\"disabled\":false,\"selected\":false,\"optionIndex\":2},{\"group\":false,\"inGroup\":false,\"text\":\"Gecko\",\"disabled\
":false,\"selected\":false,\"optionIndex\":3},{\"group\":false,\"inGroup\":false,\"text\":\"Basecamp\",\"disabled\":false,\"selected\":false,\"optionIndex\":4}]}","min":"","max":""} 

[performing the edge swipe]

I/GeckoConsole(27764): Content JS LOG: +++ something from chrome  {"type":"inputmethod-contextchange","inputType":"blur"} 
I/GeckoConsole(28943): Content JS LOG: +++ something from chrome  {"type":"inputmethod-contextchange","inputType":"select-one","value":"","choices":"{\"multiple\":false,\"choices\":[{\"group\":
false,\"inGroup\":false,\"text\":\"Mozilla\",\"disabled\":false,\"selected\":false,\"optionIndex\":0},{\"group\":false,\"inGroup\":false,\"text\":\"B2G\",\"disabled\":false,\"selected\":false,\
"optionIndex\":1},{\"group\":false,\"inGroup\":false,\"text\":\"Gaia\",\"disabled\":false,\"selected\":false,\"optionIndex\":2},{\"group\":false,\"inGroup\":false,\"text\":\"Gecko\",\"disabled\
":false,\"selected\":false,\"optionIndex\":3},{\"group\":false,\"inGroup\":false,\"text\":\"Basecamp\",\"disabled\":false,\"selected\":false,\"optionIndex\":4}]}","min":"","max":""}

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

It might be because we mistakenly focus the original appWindow(UITest) while doing swipe navigation; could you check who is calling BrowserMixin.focus during the transition?

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

Okay I know what happen. When value selector closes, it will call this.app.focus() to make the focus back to the app mozbrowser iframe; Tim had tried to remove this but caused some regression so we took it back.

The fix should be simple: before calling this.app.focus(), check if this.app.isActive(). I think this could be done in BrowserMixin.focus or ValueSelector.

Lemme know if you need me to fix it.

Comment 21

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Comment 22

[Etienne Segonzac (:etienne)]

(In reply to Autolander from comment #21)
> Created attachment 8579362 [details] [review]
> [gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Just want to run the tests, not finished yet.

Comment 23

[Etienne Segonzac (:etienne)]

Comment on attachment 8579362 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

What do you think?
This is preventing:
- hiding the value selector when an edge zone is tapped
- the injection of a bad value selector after an edge swipe

But I'm not sure if it's breaking anything (if it does it's not covered by a test :))

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8579362 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

But I doubt about removing 'blur' event because I think it's the only way to hide the value selector...?

NI Tim/Rudy to confirm.

If it's remove-able, do we need https://github.com/etiennesegonzac/gaia/blob/bug-1112524/apps/system/js/value_selector/value_selector.js#L179 ?

Comment 25

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8579362 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Hmmm....Changed my mind about isActive() part ;)

Comment 26

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

We should not remove the blur part of the code. It's possible the app programmatically remove the focus and we would need to respond to that.

Comment 27

[Etienne Segonzac (:etienne)]

Comment on attachment 8579362 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

I thought we were bluring because the system app stole the focus, but it was because we were forwarding events the the frame below.

Makes for a much cleaner fix :)

Comment 28

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8579362 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Thanks!

Comment 29

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Comment 30

[Etienne Segonzac (:etienne)]

Comment on attachment 8583026 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

Opened a new pull request to (hopefully) get the try-server-hook to see it.
Carrying the r+.

Comment 31

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 32

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 33

[Etienne Segonzac (:etienne)]

On master: https://github.com/mozilla-b2g/gaia/pull/29137

Comment 34

[Hermes Cheng (inactive after July 27, 2015)]

Please also uplift to 2.2 since it is a 2.2 blocker. Thanks.

Comment 35

[Elie]

This issue Verified successfully on flame3.0：
Reproduce rate 0/5
Build ID               20150326160206
Gaia Revision          525c341254e08f07f90da57a4d1cd5971a3cc668
Gaia Date              2015-03-26 16:34:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/59554288b4eb
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2

Comment 36

[Etienne Segonzac (:etienne)]

Comment on attachment 8583026 [details] [review]
[gaia] etiennesegonzac:bug-1112524 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): edge gestures
[User impact] if declined: modal value selector displayed on the wrong app :/
[Testing completed]: all value selectors in the UI test app + this STR with the settings app
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Comment 37

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.2 uplift.

Comment 38

[GitHub Autolander Bot]

Attachment: [gaia] etiennesegonzac:bug-1112524-v2.2 > mozilla-b2g:v2.2

Comment 39

[Etienne Segonzac (:etienne)]

New PR, test running...

Comment 40

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/8494d0ea2c9693a8e1e736b29be542c9c6d180aa

Comment 41

[Elie]

This issue verified successfully on flame2.2:
Flame 2.2:
Build ID               20150402002500
Gaia Revision          1ceca464053dee4a8bf10ea5abeef724d68c2ff2
Gaia Date              2015-04-01 09:49:30
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/427b4da96714
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Reproduce rate         0/5

Comment 42

[Lance（leave from Mozilla）]

Hi Autolander,

The bug verified fail in latest Flame 2.2, and we have build new Flame 2.2 Image of patch(comment 38), but the bug still exist.

Fail rate:5/5
Found time:16:00
See attachment:1600.mp4 and logcat_1600.txt

Flame 2.2 version:
Build ID               20150402002500
Gaia Revision          1ceca464053dee4a8bf10ea5abeef724d68c2ff2
Gaia Date              2015-04-01 09:49:30
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/427b4da96714
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150402.035057
Firmware Date          Thu Apr  2 03:51:09 EDT 2015
Bootloader             L1TC000118D0

Flame 2.2(Build):
Build ID               20150403145448
Gaia Revision          022eeb91197ba4a9adfd67bd6db5aa03cc69eb31
Gaia Date              2015-04-03 04:13:03
Gecko Revision         n/a
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.rose.20150310.100223
Firmware Date          Tue Mar 10 10:02:48 CST 2015
Bootloader             L1TC000118D0

Comment 43

[Lance（leave from Mozilla）]

Attachment: logcat_1600.txt

Comment 44

[Lance（leave from Mozilla）]

Attachment: 1600.MP4

Comment 45

[Hermes Cheng (inactive after July 27, 2015)]

Etienne, could you please take a look for 2.2? Thanks.

Comment 46

[Etienne Segonzac (:etienne)]

Comment on attachment 8560201 [details] [review]
Bug-1112524-fx

This bug had 2 patches (one more reason to always open follow up bugs I guess).

This initial patch needs to be uplifted to 2.2 too, otherwise the bug is still happening as Comment 42 states.

Comment 47

[Etienne Segonzac (:etienne)]

v2.2 PR of the missing patch (needed rebasing) -> https://github.com/mozilla-b2g/gaia/pull/29389

Comment 48

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/d161706bbd8e8615f6b39ea5aaab4a7b59fdd624

Comment 49

[Hermes Cheng (inactive after July 27, 2015)]

Lance, could you verify 2.2?

Comment 50

[Lance（leave from Mozilla）]

The problem is verified pass on latest Flame 2.2 build.

Fail rate:0/5
See attachment:1126.mp4

Device: Flame 2.2 version(Pass):
Build ID               20150409162502
Gaia Revision          df0e04acad7c8c993f6ffe07b0ccb0ec20ee50bb
Gaia Date              2015-04-09 22:35:05
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/091b1cc1240b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150409.200542
Firmware Date          Thu Apr  9 20:05:54 EDT 2015
Bootloader             L1TC000118D0

Leaving verifyme for 2.1 uplift/verification.

Comment 51

[Lance（leave from Mozilla）]

Attachment: 1126.MP4

Comment 52

[Norry.L.F(Leave from Mozilla)]

Hi Josh,

This bug is a regression. Could you check whether to uplift to flame 2.1? 
Thanks.

Comment 53

[Josh Cheng [:josh]]

Hi Etienne,
Could you help to raise 2.1 Uplift approval as this is regression issue?
Thanks!

Comment 54

[Etienne Segonzac (:etienne)]

(In reply to Josh Cheng [:josh] from comment #53)
> Hi Etienne,
> Could you help to raise 2.1 Uplift approval as this is regression issue?
> Thanks!

Not sure what the process is here and what's expected of me.
The risky-ness of the patch was described for the 2.2 approval (Comment 36).

Isn't it a pure relman issue now?