1173754 - Add restrictSearchesToken to allow to use the locationbar as the search bar

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Marco Bonardo [:mak]]

We should add a restrict token to get only search suggestions, this way the locationbar can be used similarly to the search bar.

This is not critical for first release, since these tokens are undiscoverable, but nice to have for advanced users.

Comment 1

[Drew Willcoxon :adw]

Attachment: patch

How's this?  Restricting search suggestions is different from restricting everything else because they don't live in the DB.  So it seems we need to either make MatchAutoCompleteFunction basically a no-op when restricting to suggestions, or better, just not access the DB at all, so that's what this does.  It skips the main queries but still gets the usual first result.

To make the test pass, I had to change the search string passed to getSuggestionController from _originalSearchString to _searchTokens.join(" ").  Otherwise the autocomplete results had $ in them.  However, when I tested the patch manually with _originalSearchString, there were no $ in the results.  So in other words, manual testing showed that using _originalSearchString and _searchTokens.join(" ") produced the same, apparently correct, results, but the former broke the test.

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8624947 [details] [diff] [review]
patch

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

::: toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
@@ +2,5 @@
>  
>  const ENGINE_NAME = "engine-suggestions.xml";
>  const SERVER_PORT = 9000;
>  const SUGGEST_PREF = "browser.urlbar.suggest.searches";
> +const SUGGEST_RESTRICT_TOKEN = "$";

better to read it from the pref, in case we neeed to change it for whatever reason (localization, accessibility, ecc).

@@ +222,5 @@
> +      }),
> +      title: ENGINE_NAME,
> +      style: ["action", "searchengine"],
> +      icon: "",
> +    }, {

very-nit: you used different style of bracing separation in the above object and here "},\n{" vs "}, {" as well as different indentation for "[{" vs "[\n{".

Comment 3

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/fx-team/rev/38c71cf56b27

(In reply to Marco Bonardo [::mak] from comment #2)
> ::: toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
> @@ +2,5 @@
> >  
> >  const ENGINE_NAME = "engine-suggestions.xml";
> >  const SERVER_PORT = 9000;
> >  const SUGGEST_PREF = "browser.urlbar.suggest.searches";
> > +const SUGGEST_RESTRICT_TOKEN = "$";
> 
> better to read it from the pref, in case we neeed to change it for whatever
> reason (localization, accessibility, ecc).

My understanding is that firefox.js prefs aren't set in the xpcshell environment.  Indeed when I try to get the pref in the test, it fails.  So I left this part.  If there's something obvious I'm missing I'd be glad to do a quick follow-up.

> very-nit: you used different style of bracing separation in the above object
> and here "},\n{" vs "}, {" as well as different indentation for "[{" vs
> "[\n{".

You're right, fixed.

The previous patch was based on my patch in bug 1176107 by the way, so I rebased before landing.

Comment 4

[Marco Bonardo [:mak]]

(In reply to Drew Willcoxon :adw from comment #3)
> https://hg.mozilla.org/integration/fx-team/rev/38c71cf56b27
> 
> (In reply to Marco Bonardo [::mak] from comment #2)
> > ::: toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
> > @@ +2,5 @@
> > >  
> > >  const ENGINE_NAME = "engine-suggestions.xml";
> > >  const SERVER_PORT = 9000;
> > >  const SUGGEST_PREF = "browser.urlbar.suggest.searches";
> > > +const SUGGEST_RESTRICT_TOKEN = "$";
> > 
> > better to read it from the pref, in case we neeed to change it for whatever
> > reason (localization, accessibility, ecc).
> 
> My understanding is that firefox.js prefs aren't set in the xpcshell
> environment.  Indeed when I try to get the pref in the test, it fails.  So I
> left this part.  If there's something obvious I'm missing I'd be glad to do
> a quick follow-up.

oh right, nevermind.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/38c71cf56b27