I noticed this while working on bug 1330462, and I think it should be somewhat easy to fix, provided we know what it should do. Steps: - Type "corgis" in the location bar - Hit the down key until you have a search suggestion selected, like "corgis near me" - Hit alt+down, which will let you cycle through one-off search engine buttons, until you're on, say, Bing - Notice that on the highlighted search suggestion line, it says something like "corgis near me -- Search with Bing" - Hit return, and notice that you searched Bing for "corgis", and not "corgis near me" The same steps in the search bar will result in searching Bind for "corgis near me." It seems we missed this bit when adapting the one-off buttons for the location bar. I feel like the best course of action is just to make the location bar match the behavior of the search bar in this regard, no? What seems to be getting in the way is this getter in urlbarBindings.xml: ``` <property name="oneOffSearchQuery"> <getter><![CDATA[ // this.textValue may be an autofilled string. Search only with the // portion that the user typed, if any, by preferring the autocomplete // controller's searchString (including handleEnterInstance.searchString). return (this.handleEnterInstance && this.handleEnterInstance.searchString) || this.mController.searchString || this.textValue; ]]></getter> </property> ``` I think we can work around this fairly easily so we keep the previous behavior so long as the user hasn't scrolled through the search suggestions.

We were discussing also about this in bug 1295458. I think it makes a lot of sense to special case Search suggestions.

just to be sure, Doug, do you still plan to work on this?

Yeah, I should be able to get something in for this pretty soon.

Comment on attachment 8875394 [details] Bug 1331736 - Use selected search suggestion with one-offs https://reviewboard.mozilla.org/r/146836/#review151240 ::: browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js:53 (Diff revision 1) > + `http://mochi.test:8888/?terms=foobar`); > + EventUtils.synthesizeKey("VK_RETURN", {}) > + await resultsPromise; > + > + gBrowser.removeTab(gBrowser.selectedTab); > +}); it would be great to also test a mouse click, so select the search suggestion with the keyboard, then click on the one-off ::: browser/base/content/urlbarBindings.xml:676 (Diff revision 1) > > <property name="oneOffSearchQuery"> > <getter><![CDATA[ > + if (this._isSearchSuggestion) { > + return this.textValue; > + } what I don't like about this approach is that it requires adding to the toolkit implementation stuff specific to the urlbar. Unfortunately we did that often, if possible we should try to limit though. Additionally, _isSearchSuggestion in this case would also be set to true when we completeselectedindex any other entry, like a url for example, not just search suggestions. That'd be wrong. There's another way to detect if the current value is a search suggestion though, that is using this.value instead of this.textValue. For search suggestions this.value will be, for example: moz-action:searchengine,{"engineName":"Google","input":"porcupine%20tree","searchQuery":"porcu","searchSuggestion":"porcupine%20tree"} you can use this._parseActionUrl(this.value) to get a parsed object, if the object is not nullcheck if action.type is searchengine, and then you can use action.input that should always be the right value to search for.

Comment on attachment 8875394 [details] Bug 1331736 - Use selected search suggestion with one-offs https://reviewboard.mozilla.org/r/146836/#review151754 r=me with a green(ish) Try run. ::: browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js:11 (Diff revision 2) > + ["browser.urlbar.suggest.searches", true]], > + }); > + let engine = await promiseNewSearchEngine(TEST_ENGINE_BASENAME); > + let oldCurrentEngine = Services.search.currentEngine; > + Services.search.moveEngine(engine, 0); > + Services.search.currentEngine = engine; Just in case, should you clear history before running the test? a previous test may have opened a page containing "foo". ::: browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js:25 (Diff revision 2) > +}); > + > +// Presses the Return key when a one-off is selected after selecting a search > +// suggestion. > +add_task(async function oneOffReturnAfterSuggestion() { > + gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser); it would be better (to avoid possible intermittents) if you'd use let tab = await BrowserTestUtils.openNewForegroundTab and await BrowserTestUtils.removeTab(tab); ::: browser/base/content/test/urlbar/browser_urlbarOneOffs_searchSuggestions.js:57 (Diff revision 2) > + await PlacesTestUtils.clearHistory(); > +}); > + > +// Clicks a one-off engine after selecting a search suggestion. > +add_task(async function oneOffClickAfterSuggestion() { > + gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser); ditto

Comment on attachment 8875394 [details] Bug 1331736 - Use selected search suggestion with one-offs https://reviewboard.mozilla.org/r/146836/#review151754 https://treeherder.mozilla.org/#/jobs?repo=try&revision=80b4c64c1f1572206b6c65a6697d76f417294288

Pushed by dothayer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f759d63d25b9 Use selected search suggestion with one-offs r=mak

This will ride the trains from 55.

I have reproduced this bug with Nightly 53.0a1 (2017-01-17) on Ubuntu 16.04, 64 bit! The Bug's fix is now verified on Latest Nightly 56.0a1. Build ID 20170613100218 User Agent Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0

I have reproduced this bug with Nightly 53.0a1 (2017-01-17) on Windows 10, 64-bit. The fix is now verified on latest Nightly 56.0a1 Build ID 20170613030203 User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0 [bugday-20170614]

Verified as fixed using Nightly 55.0a1 (Build ID: 20170612030208) on Windows 10 x64, Ubuntu 16.04 and Mac OS X 10.12.

Simona, why did you mark this as verified disabled? It is actually enabled.

(In reply to Panos Astithas [:past] (please needinfo?) from comment #17) > Simona, why did you mark this as verified disabled? It is actually enabled. Sorry Panos, it was by mistake. Set the right tracking flag. Thanks for the notice.

Thank you!