Closed Bug 1158280 Opened 6 years ago Closed 6 years ago

Move search suggestions disabled search primary engine above the awesomescreen results

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1158275

People

(Reporter: mcomella, Assigned: sebastian)

References

Details

Attachments

(1 obsolete file)

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?").
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
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.
Attachment #8614278 - Flags: review?(michael.l.comella)
Depends on: 1158275
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.
Attachment #8614278 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8614278 [details] [diff] [review]
1158280_primary_search_engine.patch

Patch obsolete: Backed into patch for bug 1158275.
Attachment #8614278 - Attachment is obsolete: true
This has been taken care of in bug 1158275.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1158275
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.