1131966 - [Search] After change search provider from search app, the suggestion list is not updated

Status: VERIFIED FIXED

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

Description

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

*Description:
After I changed the search engine from search app (e.g.: tapping "Google Search" to change the search provider), the suggestion list keeps the original results but not update to the list provided from the new search engine.

*STR:
1. tap rocketbar to enter search app
2. input some characters (e.g. "bus)
3. tap header of search suggestion (e.g. "Google Search")
4. Change another search engine (e.g. "Yahoo)

*Expected result:
After step 4, the search suggestion list is updated. The result is from the new search engine (e.g. Yahoo at step 4).

*Actual result:
After step 4, the search suggestion list does not change.

*Env info:
Build ID               20150210162504
Gaia Revision          b6ca0aed54fb098f8c2ca2711a917ac351fc7380
Gaia Date              2015-02-10 18:20:34
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0f4ebef7aeb1
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150210.194305
Firmware Date          Tue Feb 10 19:43:15 EST 2015
Bootloader             L1TC000118D0

Comment 1

[Dale Harvey (:daleharvey)]

Yeh I was thinking this should retrigger, will take it

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] daleharvey:1131966 > mozilla-b2g:master

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8562804 [details] [review]
[gaia] daleharvey:1131966 > mozilla-b2g:master

R- only because it breaks when changing providers after clearing the search result. I left a comment on github.

Comment 4

[Dale Harvey (:daleharvey)]

Comment on attachment 8562804 [details] [review]
[gaia] daleharvey:1131966 > mozilla-b2g:master

Fixed it to correctly handle clear, good catch thanks

(found another bug with clear as well)

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8562804 [details] [review]
[gaia] daleharvey:1131966 > mozilla-b2g:master

This should probably come with a test, but this patch looks good. Thanks!

Comment 6

[Dale Harvey (:daleharvey)]

The changes to the test ensure this is tested, previously you needed to do another search to trigger the other provider, now you dont

Comment 7

[Dale Harvey (:daleharvey)]

The try run for this is very broken, hopefully based on a bad commit, rebased and will land when green

Comment 8

[Ben Francis [:benfrancis]]

Comment on attachment 8562804 [details] [review]
[gaia] daleharvey:1131966 > mozilla-b2g:master

Thanks for getting to this so quickly Kevin.

Comment 10

[Dale Harvey (:daleharvey)]

Finally got a green run, had to restart due to an unrelated failure - https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=106f455e3bbe

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Reverted for frequent Gij failures. In addition to the Gij(3) failure your Gaia Try run had, Gij(5) was also hitting bug 1134953 extremely frequently.
Master: https://github.com/mozilla-b2g/gaia/commit/e4f7c67378e33e83f88d38ddb4a6c2cabf1423c3

Comment 12

[GitHub Autolander Bot]

Attachment: [gaia] daleharvey:1131966-redux > mozilla-b2g:master

Comment 13

[Dale Harvey (:daleharvey)]

I cant reproduce any failures locally or see why these changes would cause any failures, going to retrigger a bunch and see if anythings up

https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=57a1c44e04a2

Comment 14

[Dale Harvey (:daleharvey)]

Kevin, wondering if you had any ideas about this, 

https://github.com/mozilla-b2g/gaia/pull/28404/files is the patch

https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=57a1c44e04a2 is a try run with a bunch of failures, they mostly look like 

03:51:26     INFO -      at Object.Search.goToResults (/builds/slave/test/gaia/apps/search/test/marionette/lib/search.js:47:17)
03:51:26     INFO -      at Object.Search.triggerFirstRun (/builds/slave/test/gaia/apps/search/test/marionette/lib/search.js:107:10)
03:51:26     INFO -      at Context.<anonymous> (/builds/slave/test/gaia/apps/search/test/marionette/places_search_test.js:39:12)

I really cant tell how this change would cause this error, I am running this in a loop locally and havent been able to trigger this failure at all

Comment 15

[Kevin Grandon :kgrandon]

I don't know off the top of my head, and this seems quite painful to debug =(

I can help out, but let's do some basic debugging first. Have we tried doing the following yet:

1 - Doing a run with NODE_DEBUG=1/VERBOSE=1 to see if there's any useful output?
2 - Dumping a screenshot before the point of failure from the system app, using something like: console.log('Screenshot: data:image/png;base64,' + client.screenshot());

Comment 16

[Dale Harvey (:daleharvey)]

Comment on attachment 8568452 [details] [review]
[gaia] daleharvey:1131966-redux > mozilla-b2g:master

Ok think I have this, it could previously run initialization twice so we needed a .ready call in there and to refactor the provider to return a single promise instead of creating new ones

There were 2 failures here and they both look existing, Gu failure looks permafail on gaia-try, the homescreen failure doesnt touch the search app at all and I can see it on master, will make sure to be green before landing.

This is similiar to the original patch, but a few changes so want another set of eyes before landing

Comment 17

[Kevin Grandon :kgrandon]

Comment on attachment 8568452 [details] [review]
[gaia] daleharvey:1131966-redux > mozilla-b2g:master

Looks good to me, thanks!

Comment 18

[Dale Harvey (:daleharvey)]

Green run no restarts @ https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=66c2f6924dda

Landed in https://github.com/mozilla-b2g/gaia/commit/323d773af7c579cc7f33fbe571cc5c8631156b15

Cheers

Comment 19

[Dale Harvey (:daleharvey)]

Comment on attachment 8568452 [details] [review]
[gaia] daleharvey:1131966-redux > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Feature Work that got backed out
[User impact] if declined: Feature bug
[Testing completed]: Automated tests added
[Risk to taking this patch] (and alternatives if risky): Little Risk
[String changes made]:

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed for the latest Nightly 3.0 and 2.2 builds.

Actual Results:  Switching search providers generated a new list of search suggestions.
	
Environmental Variables:
Device: Flame 3.0 KK (319 MB) (Full Flash)
BuildID: 20150304010324
Gaia: 3fc0ac309f5fb0c1fe82c12223b955a4efce27e6
Gecko: c5b90c003be8
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2 KK (319 MB) (Full Flash)
BuildID: 20150304002529
Gaia: 8b4b3e4b7e7c308764f71542437fd60625ac6b75
Gecko: 2cb52b7cda5a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Note:  I encountered bug 1125131 while switching to Yahoo on 2.2, but was able to confirm using the other search engines.