Closed Bug 1019156 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3 unaffected, b2g-v1.3T fixed, b2g-v1.4 unaffected)

RESOLVED FIXED
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3 --- unaffected
b2g-v1.3T --- fixed
b2g-v1.4 --- unaffected

People

(Reporter: mclemmons, Assigned: chens)

References

()

Details

(Keywords: regression, Whiteboard: [tarako-exploratory])

Attachments

(3 files, 1 obsolete file)

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
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
Does this reproduce on 1.3?
Keywords: qawanted
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
Keywords: qawanted
Amir, could you take a look at this please?
Flags: needinfo?(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)
Flags: needinfo?(amirn) → needinfo?(mclemmons)
Amir, I tried reverting the patch in my own local repo and I still get the same issue.

commit 0c4290919dde2939a4de07bdc62e4776efe1f6f4
Author: nhirata <nhirata.bugzilla@gmail.com>
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.
Flags: needinfo?(mclemmons)
Keywords: regression
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
Broken by bug 1014272.

Tim - Can someone on your team look into this?
Blocks: 1014272
Flags: needinfo?(timdream)
Chens from Evelyn's team will help out. Thanks.
Flags: needinfo?(timdream) → needinfo?(shchen)
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.
Flags: needinfo?(shchen) → needinfo?(fabrice)
Attached file 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.
Attachment #8434890 - Flags: feedback?(timdream)
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.
Attachment #8434890 - Flags: feedback?(timdream) → feedback?(evyatar)
Assignee: nobody → shchen
Status: NEW → ASSIGNED
1.3T+ for regression
blocking-b2g: 1.3T? → 1.3T+
Attachment #8434890 - Flags: feedback?(evyatar) → feedback?(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.
Attachment #8434890 - Flags: feedback?(amirn) → feedback+
Attached file WIP-2 (obsolete) —
Attachment #8434890 - Flags: review?(amirn)
Attachment #8436639 - Flags: review?(amirn)
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 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.
Attachment #8436639 - Flags: review?(amirn) → review-
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 :/
(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.
Flags: needinfo?(bugs)
ni? Amir as a reminder to update the bug when the device is received. thanks
Flags: needinfo?(amirn)
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.
Flags: needinfo?(amirn)
Attachment #8434890 - Flags: review?(amirn) → review?(ran)
(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.
Flags: needinfo?(bugs)
(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?
Flags: needinfo?(fabrice) → needinfo?(bugs)
Comment on attachment 8434890 [details] [review]
WIP

Back to Amir due to PTO
Attachment #8434890 - Flags: review?(ran) → review?(amirn)
(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?
(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++)
Flags: needinfo?(bugs)
Comment on attachment 8434890 [details] [review]
WIP

Add comments in this patch
Attachment #8434890 - Flags: review?(ehung)
Attachment #8436639 - Attachment is obsolete: true
(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 on attachment 8434890 [details] [review]
WIP

As a workaround, the patch works and makes sense to me.
Attachment #8434890 - Flags: review?(ehung) → review+
ni Fabrice for my comment 29.
Flags: needinfo?(fabrice)
Sure Evelyn, go ahead.
Flags: needinfo?(fabrice)
See Also: → 1034799
Follow up bug in: bug 1034799.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8434890 [details] [review]
WIP

already landed
Attachment #8434890 - Flags: review?(amirn)
You need to log in before you can comment on or make changes to this bug.