1019156 - [B2G][Tarako][Everything.me]History bar results not selectable

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Montoya Clemmons (MClemmons)]

Attachment: Firewatch E.me search history failure

User enters several Everything.me search bar topics to generate history. Once there are several topics in the subcategory section, user attempt to select any of those options causes the bar to vanish and not call up the selected search criteria. User is forced to re-enter a search term even if it is a duplicate entry. 

Prerequisites:
1) Conduct at least three searches to build search history

Repro Steps: 
1) Update a Tarako to BuildID: 20140602014001
2) Tap Everything.me search bar
3) Tap any of the prior search history choices
4) Observe device behavior


Actual: 
The selected option vanishes with the remainder of the subcategory results. The screen returns to its prior state of the I'm thinking of... search bar and the core four icons. 

Expected: 
The selected option generates App results on the E.me page 


Notes: 
Repro frequency: (5/5, 100%) 

See attached: video = https://www.youtube.com/watch?v=8lf5Bm8WrrE, logcat, firewatch

1.3T Environmental Variables:
Device: Tarako 1.3T
BuildID: 20140602014001
Gaia: 335486c42498fa7a93c21e4d6121199728602ab8
Gecko: 55e4d83019e5
Version: 28.1
Firmware Version: sp6821a-gonk-4.0-5-12
User Agent: Mozilla/5.0 (Mobile; rv:28.1) Gecko/28.1 Firefox/28.1

Comment 1

[Montoya Clemmons (MClemmons)]

Attachment: Logcat E.me history search bar problem

Comment 2

[Montoya Clemmons (MClemmons)]

This issue does not reproduce on Flame 1.4 following the STR from Comment 0. Search history is selectable every time. 

1.4F Environmental Variables:
Device: Flame 1.4F
BuildID: 20140529000204
Gaia: 7bc1c15c67661a0b8e35f18f15a9d03d1d2cfcd5
Gecko: 2181cac4d0fc
Version: 30.0
Firmware Version: v10G-2
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 3

[Jason Smith [:jsmith]]

Does this reproduce on 1.3?

Comment 4

[Montoya Clemmons (MClemmons)]

This issue does not reproduce on Buri 1.3 following the STR from Comment 0. Search history is selectable every time. 

1.3 Environmental Variables: 
Device: Buri 1.3 MOZ 
BuildID: 20140529024024 
Gaia: 5bd226b03a2d63dfe9df204f7c0afb9984e8fd42 
Gecko: 42ef074f380f 
Version: 28.0 
Firmware Version: v1.2-device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Amir, could you take a look at this please?

Comment 6

[Amir Nissim (:amirn)]

can you please try backing out commit https://github.com/dwi2/gaia/commit/ad049bb9cc6ba57839649726560955592352023a and see if the issue is resolved?

This patch is 1.3t only and might be the cause for this issue (landed in bug 999953)

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Amir, I tried reverting the patch in my own local repo and I still get the same issue.

commit 0c4290919dde2939a4de07bdc62e4776efe1f6f4
Author: nhirata <[e-mail]>
Date:   Tue Jun 3 09:27:54 2014 -0700
    Revert "Bug 999953 - Preload searchbar and make evme icons at correct positi
    This reverts commit ad049bb9cc6ba57839649726560955592352023a.

I guess what might help is a regression range.  Marking the bug as such for trying to find the regression window.

Comment 8

[Joshua Mitchell (Inactive)]

Tarako Regression Window:

Last Working:
Environmental Variables:
Device: Tarako 1.3T
BuildID: 20140521222847
Gaia: f64d0237b49dab5eba5dc82b2a6d816e1e481f62
Gecko: 3d434185327b
Version: 28.1
Firmware Version: sp6821a-gonk-4.0-5-12

First Broken:
Environmental Variables:
Device: Tarako 1.3T
BuildID: 20140521232821
Gaia: 2c0390f6f4fa0076092c7a71731a99d4e4d17874
Gecko: 31d6c30f70f2
Version: 28.1
Firmware Version: sp6821a-gonk-4.0-5-12

Last Working Gaia First Broken Gecko: Issue DOES not reproduce 
Gaia: f64d0237b49dab5eba5dc82b2a6d816e1e481f62
Gecko: 31d6c30f70f2

First Broken Gaia Last Working Gecko: Issue DOES reproduce (Indicating a Gaia Issue)
Gaia: 2c0390f6f4fa0076092c7a71731a99d4e4d17874
Gecko: 3d434185327b

Tarako Pushlog: https://github.com/mozilla-b2g/gaia/compare/f64d0237b49dab5eba5dc82b2a6d816e1e481f62...2c0390f6f4fa0076092c7a71731a99d4e4d17874

Comment 9

[Jason Smith [:jsmith]]

Broken by bug 1014272.

Tim - Can someone on your team look into this?

Comment 10

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

Chens from Evelyn's team will help out. Thanks.

Comment 11

[Sherman Chen [:chens] (inactive)]

Update findings we got so far, in normal case we should have one click event from search history list and one blur event from search bar. After bug 1014272 introduced in-process homescreen to gaia, we can only get blur event from search bar, while click event from search history list is missing. This cause the situation described in comment 0, not sure if this is related to in-process event handling in gecko. NI Fabrice to see if there's any suggestion on this one.

Comment 12

[Sherman Chen [:chens] (inactive)]

Attachment: WIP

Gaia workaround for this specific issue on 1.3t in-proc homescreen, this will fire another click event respond to the missing one. Not sure if this the right way to go, feedback needed.

Comment 13

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

Comment on attachment 8434890 [details] [review]
WIP

Sorry for the delay. I will defer the feedback to e.me owners instead.

The missing event sounds really strange... I recommend we dug further and maybe come up with a solution w/o changing code in e.me.

Comment 14

[Joe Cheng [:jcheng] (please needinfo)]

1.3T+ for regression

Comment 15

[Amir Nissim (:amirn)]

Comment on attachment 8434890 [details] [review]
WIP

I agree with Tim (comment 13)

There might be other side affects we have yet to discover, so I think the right way to go would be to find the cause for the missing clicks and fix it.

The patch looks ok if you want to land a quick fix for this issue.
I think it would help if we added a comment there to clear things up though.

I am unable to test it since I don't have a Tarako device available.

Comment 16

[Sherman Chen [:chens] (inactive)]

Attachment: WIP-2

Comment 17

[Sherman Chen [:chens] (inactive)]

Create another WIP in respond to the issue. 
I think the root cause is because search history will become hidden on search bar blur. Thus we can only get blur event from search bar and not able to get click event from search history item. 

Refer to [1], [2] we can found that blur event is trying to hide helper header which contains search history, and combining the style of keyboard hidden and a empty search bar, click event won't be triggered because it is already hidden. And WIP-2 choose another route to delay hiding helper header so that we can still have click event available.

[1]: https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/homescreen/everything.me/js/Brain.js#L190-L191
[2]: https://github.com/mozilla-b2g/gaia/blob/v1.3t/apps/homescreen/everything.me/modules/Helper/Helper.css#L7-L9

Comment 18

[Amir Nissim (:amirn)]

Comment on attachment 8436639 [details] [review]
WIP-2

I still think we should investigate further to find what caused this regression. Is it a platform issue? Why does it happen only on Tarakos?

As Tim suggested in comment 13, it would be safer if we found a solution that does not involve changing e.me code (which did not change hence did not cause this bug)

This patch feels too risky to me, and if you need a quick fix for this issue I suggest you use the first patch instead.

Thanks.

Comment 19

[Amir Nissim (:amirn)]

Comment on attachment 8434890 [details] [review]
WIP

I should get a Tarako device in the next few days. Till then, can't really test this. Sorry :/

Comment 20

[Sherman Chen [:chens] (inactive)]

(In reply to Amir Nissim (Everything.me) from comment #18)
> I still think we should investigate further to find what caused this
> regression. Is it a platform issue? Why does it happen only on Tarakos?
> 
> As Tim suggested in comment 13, it would be safer if we found a solution
> that does not involve changing e.me code (which did not change hence did not
> cause this bug)

This happens only on Tarakos after homescreen in process, not sure if event handling mechanism has any difference between in process and out of process, needinfo :smaug to see if there's any suggestion on this one.

Comment 21

[Joe Cheng [:jcheng] (please needinfo)]

ni? Amir as a reminder to update the bug when the device is received. thanks

Comment 22

[Ran Ben Aharon [:ranbena] EverythingMe]

Got a Tarako device working finally.
Click works, nothing else seems affected.

This seems to have exposed a similar issue - swiping the history bar right/left generates a click on it.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Sherman Chen [:chens] from comment #20)
> This happens only on Tarakos after homescreen in process, not sure if event
> handling mechanism has any difference between in process and out of process,
> needinfo :smaug to see if there's any suggestion on this one.

in-process and out-of-process may have rather different behavior. They use different widget level code, and
only some events are forwarded to child process.

So someone should perhaps debug which events we get in out-of-process and which in in-process case.

Comment 24

[[:fabrice] Fabrice Desré]

(In reply to Olli Pettay [:smaug] from comment #23)
> (In reply to Sherman Chen [:chens] from comment #20)
> > This happens only on Tarakos after homescreen in process, not sure if event
> > handling mechanism has any difference between in process and out of process,
> > needinfo :smaug to see if there's any suggestion on this one.
> 
> in-process and out-of-process may have rather different behavior. They use
> different widget level code, and
> only some events are forwarded to child process.
> 
> So someone should perhaps debug which events we get in out-of-process and
> which in in-process case.

Comment 11 hints that we miss a click event when running the homescreen in-process. I'm happy to debug further on a tarako, but can you tell me where to look into?

Comment 25

[Ran Ben Aharon [:ranbena] EverythingMe]

Comment on attachment 8434890 [details] [review]
WIP

Back to Amir due to PTO

Comment 26

[Evelyn Hung]

(In reply to Amir Nissim [on PTO until Jul 6] (:amirn) from comment #15)
> Comment on attachment 8434890 [details] [review]
> WIP
> 
> I agree with Tim (comment 13)
> 
> There might be other side affects we have yet to discover, so I think the
> right way to go would be to find the cause for the missing clicks and fix it.
> 
> The patch looks ok if you want to land a quick fix for this issue.
> I think it would help if we added a comment there to clear things up though.
> 
> I am unable to test it since I don't have a Tarako device available.

Hi Amir, since it's a 1.3t blocker, I feel we can add comments and take this quick fix if it works. The patch will only be landed to 1.3t branch. Are you okay with this proposal?

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Fabrice Desré [:fabrice] from comment #24)
> Comment 11 hints that we miss a click event when running the homescreen
> in-process. I'm happy to debug further on a tarako, but can you tell me
> where to look into?
Hmm, I thought I had commented here.

click is dispatched in EventStateManager if mousedown/up happen on the same element.
So check at least what is the originalTarget for mousedown/up.
(You may need to check that in C++)

Comment 28

[Sherman Chen [:chens] (inactive)]

Comment on attachment 8434890 [details] [review]
WIP

Add comments in this patch

Comment 29

[Evelyn Hung]

(In reply to Olli Pettay [:smaug] from comment #27)
> (In reply to Fabrice Desré [:fabrice] from comment #24)
> > Comment 11 hints that we miss a click event when running the homescreen
> > in-process. I'm happy to debug further on a tarako, but can you tell me
> > where to look into?
> Hmm, I thought I had commented here.
> 
> click is dispatched in EventStateManager if mousedown/up happen on the same
> element.
> So check at least what is the originalTarget for mousedown/up.
> (You may need to check that in C++)

Thanks :smaug providing the debug information.

Hi Fabrice,
It's a 1.3t blocker, do you feel okay to workaround in Gaia first, and open a follow up bug for finding the root cause?

Comment 30

[Evelyn Hung]

Comment on attachment 8434890 [details] [review]
WIP

As a workaround, the patch works and makes sense to me.

Comment 31

[Evelyn Hung]

ni Fabrice for my comment 29.

Comment 32

[[:fabrice] Fabrice Desré]

Sure Evelyn, go ahead.

Comment 33

[Sherman Chen [:chens] (inactive)]

Merged to v1.3t
https://github.com/mozilla-b2g/gaia/commit/df0ba9a12ece8d342581cfa2b5607641bf12f3c3

Comment 34

[Sherman Chen [:chens] (inactive)]

Follow up bug in: bug 1034799.

Comment 35

[Amir Nissim (:amirn)]

Comment on attachment 8434890 [details] [review]
WIP

already landed