1036671 - [B2G][Homescreen][Rocketbar] - Rocketbar results can't be selected after previously using the Rocketbar.

Status: VERIFIED FIXED

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

Description

[Martin Shuman [:Marty]]

Attachment: logcat.txt

Description:
If the user has already used the Rocketbar and selected a search result, they will not be able to select any future Rocketbar search results.

Repro Steps:
1) Update a Flame device to BuildID: 20140708000322
2) Make sure the Flame device is set to 273mb.
3) Search "Browse" in the Rocketbar, then select the Browser
4) Tap the home button, and search "Video" in the Rocketbar
5) Try to select the Video search result

Actual:
Rocketbar search results are only selectable the first time.

Expected:
Rocketbar search results are always selectable.

Environmental Variables:
Device: Flame 2.0 MOZ ril (273mb)
BuildID: 20140708000322
Gaia: e935f4ff190b76c70d9b2af8856c542a6e4a7546
Gecko: 3f9d7a3a0b7b
Version: 32.0a2
Firmware Version: v122
2.0 - Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Keywords: Rocketbar, Homescreen, Search

Notes: Restarting the phone will temporarily undo the issue.

Repro frequency: 100%
Link to failed test case:
See attached: Video, logcat

---------------------------------------

This issue DOES occur on Flame v2.1 (273mb)

Environmental Variables:
Device: Flame Master
BuildID: 20140709040203
Gaia: 0f9f11d0a6dadb3ea27160204bbe911c1ad69a6f
Gecko: 196d05832e12
Version: 33.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Rocketbar search results are only selectable from the first search.

----------------------------------------

This issue does NOT occur on Flame 2.0 (512mb), Buri v2.0, Buri v2.1, Open-C v2.0, Open-C v2.1

Environmental Variables:
Device: Flame 2.0 MOZ ril (512mb)
BuildID: 20140708000322
Gaia: e935f4ff190b76c70d9b2af8856c542a6e4a7546
Gecko: 3f9d7a3a0b7b
Version: 32.0a2 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Buri Master
Build ID: 20140709073020
Gaia: c394b7b4205b6f1a6ca44915fc08650f3ad127ec
Gecko: 2d88803a0b9c
Version: 33.0a1 (Master)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Environmental Variables:
Device: Buri 2.0 MOZ ril
Build ID: 20140709063007
Gaia: 1774027323bb072b4ebdfea9883572bcf2535c87
Gecko: 11b6493a7d8f
Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Open_C Master
Build ID: 20140708040218
Gaia: 740faa5d0060fb218b407cf224330654ddf833a5
Gecko: 465280604ea6
Version: 33.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0


Environmental Variables:
Device: Open_C 2.0 MOZ ril
Build ID: 20140708000322
Gaia: e935f4ff190b76c70d9b2af8856c542a6e4a7546
Gecko: 3f9d7a3a0b7b
Version: 32.0a2 (2.0)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

All Rocketbar results are selectable for all searches.

---------------------------------------------

This is a new issue for 2.0, and was not implemented in 1.4.

Comment 1

[Martin Shuman [:Marty]]

I think this issue might be related to restoring the previous Rocketbar search results.  I hit the same problem after recieving an SMS and being returned to the Homescreen.  When reopening the Rocketbar, the search field is empty, unlike how it behaves on Flame 2.0 512mb, or Buri 2.0, where it repopulates with the previous search term and search results.

Comment 2

[Kevin Grandon :kgrandon]

Martin - can you tell me if you had geolocation enabled or disabled? I'm wondering if you're running into the geolocation bug that is causing problems in this 273mb configuration. Thanks!

Comment 3

[Martin Shuman [:Marty]]

I have this happen when Geolocation is both enabled and disabled, no differences observed either way.

Comment 4

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Hey guys - either of you available to review this one? The problem is that we were not calling destroy() after an OOM. This resulted in multiple container nodes in the markup, overlaying the current container, and not allowing clicks through.

Comment 5

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

Comment on attachment 8454037 [details] [review]
Github pull request

Odd.
AppWindow.prototype._handle_mozbrowsererror should call destroy()(in kill()) for you.
Could you investigate why destroy() is not called? This will help for future window management code maintenance.

Comment 6

[Kevin Grandon :kgrandon]

Hey Alive - it looks like we get into this block and destroy never gets called because isActive returns true for the search app, but the _closed event is never fired on the iframe: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L420

Any preference on how to solve it? (You can also steal this bug if you'd like)

Comment 7

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

Stealing ;)

Comment 8

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

Attachment: patch for master

After reading the rocketbar.js I am sure we need someone to clean it because of the state control seems weird now.

The fix here is avoiding rewrite it with
1. Use terminated event
2. requestClose to map close when search window crashed
3. Use immediate transition instead of useless zoom-in/zoom-out
4. Use open/close on searchWindow instead of _setVisible when launching/closing rocketbar
5. Check searchWindow.isDead before opening it. It's because in terminated event handler it will call clear->handleInput->showResult->open the searchWindow again. This is terrible :/
The problem is even Kevin is not sure what to do here so the safest way is not changing the logic of terminated handler but have a check.

Comment 9

[Kevin Grandon :kgrandon]

Comment on attachment 8455254 [details] [review]
patch for master

The code looks fine to me, I haven't verified yet, but will take a look on device soon. F+ from me though!

Comment 10

[Etienne Segonzac (:etienne)]

Comment on attachment 8455254 [details] [review]
patch for master

Haven't looked at the weird state control, but this patch makes sense and I'm happy to remove the _setVisible calls from rocketbar.js anyway :)

Comment 11

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

https://github.com/mozilla-b2g/gaia/commit/12bb8ef666d769535d2a72a76fae217fb7b22f1c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/f72b8071f3784998135d4afd94a3de1b2184910b

Comment 13

[Andrew Stole [:astole]]

Verified fixed on latest 2.0. Has not been verified on 2.1 yet.

2.0 Environmental Variables:
Device: Flame 2.0
BuildID: 20140716000201
Gaia: 5f8b1b8a2da9e3b531eee817a669f57fa4d9b9c6
Gecko: 913827496f65
Version: 32.0a2
Firmware Version: v122

Comment 14

[Martin Shuman [:Marty]]

Verified fixed on Flame 2.1.
Rocketbar search results are always selectable.

Environmental Variables:
Device: Flame 2.1
Build ID: 20140905000202
Gaia: 95e9b099aa89ded133e44014dd40b19dc0193c01
Gecko: 92a6bbdfd945
Version: 34.0a2 (2.1)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0