Closed Bug 896560 Opened 11 years ago Closed 11 years ago

[fig] Fix and re-enable testSearchSuggestions

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: Margaret, Unassigned)

References

Details

(Whiteboard: [fixed-fig], abouthome-hackathon)

Attachments

(2 files)

Updating this test will require some re-working for the new search suggestions UI we built (the current test depends on the AwesomeBarTabs implementation).
Attachment #785054 - Flags: review?(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.
Attachment #785054 - Flags: review?(margaret.leibovic) → review+
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!
Attachment #785055 - Flags: review?(margaret.leibovic) → review+
(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.
Pushed:
http://hg.mozilla.org/projects/fig/rev/1a6e28e051e0
http://hg.mozilla.org/projects/fig/rev/d9ff3e3468ee
Whiteboard: abouthome-hackathon → [fixed-fig], abouthome-hackathon
https://hg.mozilla.org/mozilla-central/rev/1a6e28e051e0
https://hg.mozilla.org/mozilla-central/rev/d9ff3e3468ee
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Depends on: 907768
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: