1158280 - Move search suggestions disabled search primary engine above the awesomescreen results

Status: RESOLVED DUPLICATE of bug 1158275

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Follow-up to bug 1137483 - doesn't actually need it to land, but we might as well let it land first.

If the search suggestions are disabled, the primary search engine appears below the awesomescreen results. This is bad from a search engine visibility perspective (i.e. search branding) and I think it makes the UI slightly less intuitive as far as search is concerned (e.g. "can I only search awesomescreen results?").

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1158280_primary_search_engine.patch

This is a follow-up patch to the one in bug 1158275. The patch in bug 1158275  only shows the primary search engine in the result list when suggestions are enabled. This is fixed with this patch and the primary search engine is moved to the top.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8614278 [details] [diff] [review]
1158280_primary_search_engine.patch

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

I notice this undoes some of the work in the dependent bug - if you find that two bugs step on each other's toes too much to write cleanly, feel free to write one changeset (or series of changesets) and post them to a single bug. In the other bug, mark it as a duplicate and write something like, "Fixed in bug ...".

A bit concerned about getSuggestEngineCount, but if you think it works as is, wfm.

::: mobile/android/base/home/BrowserSearch.java
@@ +891,5 @@
>              if (TextUtils.isEmpty(mSearchTerm)) {
>                  return resultCount;
>              }
>  
> +            return resultCount + (mSearchEngines.size() > 0 ? 1 : 0);

nit: `mSearchEngines.size() > 0 ? 1 : 0` could probably also be a helper function.

What does this mean for `getSuggestEngineCount`? Is using it some places but not here inconsistent? Should we be using your calculation everywhere instead? Can we get rid of `getSuggestEngineCount`? Maybe that's the extent of the cleanup I referred to in the last bug.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8614278 [details] [diff] [review]
1158280_primary_search_engine.patch

Patch obsolete: Backed into patch for bug 1158275.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

This has been taken care of in bug 1158275.