Drew Willcoxon :adw Reporter
	Description • 10 years ago

Bug 612453 added search suggestions to about:home/newtab.  It used an almost entirely custom implementation even though we already have search suggestions in the main chrome search bar.  We should use the same implementation if/where possible.  Specifically we should investigate using the usual search autocomplete back-end and XUL popup in about:home/newtab.

I have a prototype patch that hardcodes nsFormFillController::GetSearchAt to return "search-autocomplete" instead of "form-history", and with that small change it basically works.  It doesn't work in an e10s window, though, so more investigation is needed there.

There are three parts to suggestions:

(1) retrieving suggestions and form history
(2) handling interaction, e.g., to show/hide the popup (keypress, input, focus, blur events, etc.)
(3) building/rendering the popup

Below, for each part I'll break down how the usual search/form autocomplete back-end and XUL popup work for html:inputs and the main search bar.

(1) html:inputs have a simple form fill popup.  How does that work?  browser.xul sets an autocompletepopup attribute on the tabbrowser that filters down to each xul:browser.  xul:browser calls attachFormFill on itself on pageshow.  In attachFormFill, if the browser has an autocompletepopup attribute, it gets an instance of nsIFormFillController and calls attachToBrowser on it.  The nsIFormFillController impl is in nsFormFillController.cpp.  nsFormFillController::AttachToBrowser adds a bunch of event listeners to the content window.  When an input is focused, nsFormFillController::Focus starts controlling that input: nsFormFillController owns an nsIAutoCompleteController, and it sets nsIAutoCompleteController.input = to the input.  On input, nsFormFillController calls handleText on its nsIAutoCompleteController, which starts a search.  The nsIAutoCompleteController impl, nsAutoCompleteController, calls getSearchAt on its input.  That ends up calling nsFormFillController::GetSearchAt, which is hardcoded to return "form-history".  That causes the provider implemented by "@mozilla.org/autocomplete/search;1?name=form-history" to be used for completions/suggestions, which is nsFormFillController, which implements nsIAutoCompleteSearch and provides completions via Satchel.

Suggestions in the main search bar are provided by "@mozilla.org/autocomplete/search;1?name=search-autocomplete", which is nsSearchSuggestions.js.  So we need to somehow hook up search-autocomplete instead of form-history to the html:inputs on about:home/newtab.

How does the main search bar do it?  Its xul:textbox is bound to the autocomplete binding in autocomplete.xml.  autocomplete implements getSearchAt and returns "search-autocomplete".

We want to keep the html:inputs in about:home/newtab rather than using XUL, so we're stuck with nsFormFillController.  So we can change nsFormFillController::GetSearchAt to return "search-autocomplete" for about:home/newtab.  How?  GetSearchAt could just check the URI of the input's document.  If that's distasteful, we could set an attribute on the about:home/newtab's xul:browsers that affects the call to nsFormFillController::AttachToBrowser -- AttachToBrowser would sniff for the presence of the attribute (filtered down to the docshell?) and then use "search-autocomplete".

We would need to make sure that once you navigate away from about:home/newtab, "form-history" is used.

(2) Handled by the various event listeners in nsFormFillController as alluded to in (1).

(3) As mentioned in (1), tabbrowser/browser has an autocompletepopup attribute.  The search bar xul:textbox also has it.  In both cases its value is PopupAutoComplete, a xul:panel in browser.xul.  nsAutoCompleteController and nsFormFillController open and close the popup.  It looks like nsAutoCompleteController fills in the popup, with a xul:tree.

The tree is styled by the browser.css's in the themes directory, e.g., http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css.  (See the /* ----- AUTOCOMPLETE ----- */ section.)

The "Suggestions" comment string that appears at the top of the search suggestions section in the main search bar's popup is part of the results returned by nsSearchSuggestions.js and SearchSuggestionController.  It ends up being returned by FormAutoCompleteResult.prototype.getCommentAt, which nsAutoCompleteController  calls to set tree cell text.  nsAutoCompleteController also calls getStyleAt, which is how the popup's separator between form history results and search provider results is drawn, along with a border in the CSS in the browser.css's.

	Drew Willcoxon :adw Reporter
	Comment 1 • 10 years ago

mconnor says that another reason he chose HTML for the about:home/newtab popup is that it supports dir="auto" so that each result in the popup is shown with its correct text direction (see also bug 1073692).  Ehsan says XUL doesn't support that and it would be hard to make it support it.

So for example, once bug 1073692 is fixed, on about:home/newtab, if you have a popup with some entries whose text is LTR and some whose text is RTL, then each will appear according to its text direction.  LTR entries will hug the left side of the popup and RTL entries will hug the right side -- all in the same popup.

But the same isn't true of the search bar's popup, and it's hard to make it behave that way.  Which is really a separate bug, but it pertains to this one, too.

Previously Mike had suggested converting the XUL popup to HTML instead of vice versa.

	Florian Quèze [:florian]
	Comment 2 • 9 years ago

Is this still relevant, given bug 1176429?

	Drew Willcoxon :adw Reporter
	Comment 3 • 9 years ago

I'm not sure, I'm not really following the remote newtab work.  Marina, will the remote newtab have a search field implemented on the client side?  If not, we could close this bug, but if so, we may still want to keep it open.

BTW since this bug was filed, Nihanth rewrote the content search UI to match the newer one-off UI (bug 1171344), but it still uses a separate implementation from the search bar's one-off UI.  Ideally they would be the same, still.

	Marina Samuel [:emtwo]
	Comment 4 • 9 years ago

Ursula is currently working on migrating Nihanth's work into remote newtab. The ongoing work is here: 
https://github.com/mozilla/remote-newtab/pull/80

I think it does make sense to be using the same autocomplete back-end as the search bar so I've made this bug blocking remote newtab for now. But using a XUL popup isn't relevant, it will all be HTML so updated the title here.