The new search field on about:newtab should provide search suggest. It should behave as the about:home search field changes happening in bug 612453.

For the record, here is the latest look and feel, from bug 612453: https://bug612453.bugzilla.mozilla.org/attachment.cgi?id=8419799

Is this different from bug 1015269?

Not really. Mike, are you still working on that one?

I didn't know the longer term goal was for about:newtab to be unprivileged, so copying the about:home stuff feels like a better plan. Will dupe forward.

Please request a11y-review from me on the patch when it's ready, too, so we can make sure this feature is accessible to keyboard and screen reader users when it lands. Look in bug 612453 for how I did it there, too. :)

Marco, can you please drop this from the current iteration? I talked to Gavin and this isn't ready yet. It needs to wait for bug 612453 as that will provide some basic infrastructure. Sorry, Mano.

Removed from Iteration 33.2

I hear that, do to refactoring going to to resolve bug 612453, that this would be relatively easy to do as well. Is that the case?

yes

Based on the patches in bug 612453. https://tbpl.mozilla.org/?tree=Try&rev=b66b8f35b8ef

There was one intermittent failure on Linux x64 Debug, when the test opens the search panel and checks it. It gets stuck waiting for the panel to open. At that time, the current engine is the suggestions engine, which this patch added, and the suggestion engine doesn't have a logo, so there's no predetermined place for the panel to popup. (When I run locally, I see it pop up near the bottom right corner of the page.) I changed the test to stop doing that if the current engine doesn't have a logo. Try looks better so far: https://tbpl.mozilla.org/?tree=Try&rev=09a2ebc1e40b

Here's the updated patch as described in the previous comment.

This is equivalent to the previous but preserves blame better. Sorry for the churn.

Comment on attachment 8462949 [details] [diff] [review] patch v2.1 Review of attachment 8462949 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments. You'll probably want to adjust the commit message too. ::: browser/base/content/newtab/newTab.css @@ +394,5 @@ > } > + > +.searchSuggestionTable { > + font: message-box; > + font-size: 16px; Why not em so it zooms? ::: browser/base/content/newtab/newTab.xul @@ +5,4 @@ > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > > <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> > +<?xml-stylesheet href="chrome://browser/content/searchSuggestionUI.css" type="text/css"?> In the dependency I just commented that this should get appended dynamically with <style scoped>. ::: browser/base/content/newtab/search.js @@ +55,5 @@ > }, > > handleEvent: function (event) { > + let methodName = "on" + event.detail.type; > + if (methodName in this) { hasOwnProperty would be somewhat safer to avoid accidentally calling one of the functions on a default event handler property e.g. if |this| was not what was expected. ::: browser/base/content/test/newtab/browser.ini @@ +34,5 @@ > searchEngine1xLogo.xml > searchEngine2xLogo.xml > searchEngine1x2xLogo.xml > + searchEngineSuggestions.xml > + searchEngineSuggestions.sjs I don't think we need a third copy of these files. You should be able to use relative paths "../"… to use the same files.

(In reply to Matthew N. [:MattN] from comment #16) > ::: browser/base/content/newtab/newTab.css > @@ +394,5 @@ > > } > > + > > +.searchSuggestionTable { > > + font: message-box; > > + font-size: 16px; > > Why not em so it zooms? So it matches the input selector at the top of the file. If by zooms you mean Cmd-+/-, it does zoom when you do that. > ::: browser/base/content/newtab/newTab.xul > @@ +5,4 @@ > > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > > > > <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> > > +<?xml-stylesheet href="chrome://browser/content/searchSuggestionUI.css" type="text/css"?> > > In the dependency I just commented that this should get appended dynamically > with <style scoped>. It didn't work unfortunately (bug 612453 comment 85). > ::: browser/base/content/newtab/search.js > @@ +55,5 @@ > > }, > > > > handleEvent: function (event) { > > + let methodName = "on" + event.detail.type; > > + if (methodName in this) { > > hasOwnProperty would be somewhat safer to avoid accidentally calling one of > the functions on a default event handler property e.g. if |this| was not > what was expected. Done. > ::: browser/base/content/test/newtab/browser.ini > @@ +34,5 @@ > > searchEngine1xLogo.xml > > searchEngine2xLogo.xml > > searchEngine1x2xLogo.xml > > + searchEngineSuggestions.xml > > + searchEngineSuggestions.sjs > > I don't think we need a third copy of these files. You should be able to use > relative paths "../"… to use the same files. Removed these copies and changed browser.ini to ../general/searchSuggestionEngine.xml and ../general/searchSuggestionEngine.sjs.

Verified using Nightly 34.0a1 20140804030205 under Win 7 64-bit, Mac OSX 10.9.4 and Ubuntu 12.10 32-bit: search suggestions are displayed on about:newtab page for all default search engines except Twitter. Logged separate issues for the bugs found.

Already verified in Nightly 34 (comment 20).

This isn't substantially different from patch v3, just updates some line numbers to apply totally cleanly. Bug 1054516 is the uplift bug for this feature. mozilla-aurora try push with this and patches for other bugs mentioned in bug 1054516: https://tbpl.mozilla.org/?tree=Try&rev=fba51f37ff40 Approval Request Comment [Feature/regressing bug #]: this bug, search suggestions in about:newtab [User impact if declined]: no search suggestions in about:newtab [Describe test coverage new/current, TBPL]: currently covered by automated tests on 34; this patch adds tests to 33; see also try link above [Risks and why]: moderate risk mediated by aforementioned testing; modifies the about:newtab page; we want about:newtab search suggestions on 33 as stated in bug 1054516 [String/UUID change made/needed]: none

Verified that the search suggestions are displayed on about:newtab page using latest Aurora 33.0a2 (20140828004002) under Win 7 64-bit, Ubuntu 13.04 32-bit and Mac OSX 10.8.5.

please see Bug 1092957