896560 - [fig] Fix and re-enable testSearchSuggestions

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Updating this test will require some re-working for the new search suggestions UI we built (the current test depends on the AwesomeBarTabs implementation).

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: (1/2) Only set SuggestClient if not defined yet

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: (2/2) Update testSearchSuggestions test for new about:home

Comment 3

[:Margaret Leibovic]

Comment on attachment 785054 [details] [diff] [review]
(1/2) Only set SuggestClient if not defined yet

Review of attachment 785054 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/home/BrowserSearch.java
@@ +397,5 @@
>                      // be null is when we're restoring after a crash. We should
>                      // never restore private tabs when that happens, so it
>                      // should be safe to assume that null means non-private.
>                      Tab tab = Tabs.getInstance().getSelectedTab();
> +                    final boolean isNotPrivate = (tab == null || !tab.isPrivate());

I feel like this could be clearer as isPrivate = (tab != null && tab.isPrivate()), then just check !isPrivate down below. But I don't feel strongly, your choice.

Comment 4

[:Margaret Leibovic]

Comment on attachment 785055 [details] [diff] [review]
(2/2) Update testSearchSuggestions test for new about:home

Review of attachment 785055 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #3)
> Comment on attachment 785054 [details] [diff] [review]
> (1/2) Only set SuggestClient if not defined yet
> 
> Review of attachment 785054 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/home/BrowserSearch.java
> @@ +397,5 @@
> >                      // be null is when we're restoring after a crash. We should
> >                      // never restore private tabs when that happens, so it
> >                      // should be safe to assume that null means non-private.
> >                      Tab tab = Tabs.getInstance().getSelectedTab();
> > +                    final boolean isNotPrivate = (tab == null || !tab.isPrivate());
> 
> I feel like this could be clearer as isPrivate = (tab != null &&
> tab.isPrivate()), then just check !isPrivate down below. But I don't feel
> strongly, your choice.

Done.

Comment 6

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/projects/fig/rev/1a6e28e051e0
http://hg.mozilla.org/projects/fig/rev/d9ff3e3468ee

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1a6e28e051e0
https://hg.mozilla.org/mozilla-central/rev/d9ff3e3468ee