Closed Bug 1187753 Opened 4 years ago Closed 3 years ago

Autocomplete popup displays wrong message when typing only a search keyword

Categories

(Firefox :: Address Bar, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.3 - Aug 10
Tracking Status
firefox42 --- fixed

People

(Reporter: arni2033, Assigned: mak)

References

Details

(Whiteboard: [unifiedcomplete][fxsearch])

Attachments

(2 files)

STR:
1.   Install latest Nightly [42.0a1 (2015-07-26)] , create new profile
2.   Visit https://youtube.com so that it would be saved in history
3.   Set "y" as keyword for (e.g.) Yahoo!
4.A) (1st scenario) type "y" in addressbar, delete selected autocompleted text "outube.com/"
4.B) (2nd scenario) type "y " in addressbar

RESULT:
In both scenarios you'll get wrong 1st autocomplete result. 

EXPECTATIONS:
Note that if you press "Enter" after Step 4, the Yahoo! search homepage will be loaded. So autocomplete popup should tell about that.

SEE ALSO:
bug 1085374. Actually, it's unclear for me why Marco Bonardo said in that bug that expected behavior is to search in default search engine. What _I_ wrote in "EXPECTATIONS" is present in Fx for ages.
I'd like to see link to the discussion of this behavior.
most or everything of this is due to bug 1172937. We'll check again after it lands.
Status: UNCONFIRMED → NEW
Depends on: 1172937
Ever confirmed: true
When only "y" or "y " (a search keyword) is typed we sate "Search with google" but then open yahoo!

This is still a bug to fix.
Flags: firefox-backlog+
Priority: -- → P3
Whiteboard: [unifiedcomplete][fxsearch]
Summary: Autocomplete popup displays wrong message when trying to search with keyword → Autocomplete popup displays wrong message when typing only a search keyword
Blocks: 1183921
Priority: P3 → P2
There are 2 things to look at:

  _matchSearchEngineAlias: function* () {
    if (this._searchTokens.length < 2)
      return false;

this was the original idea, that the keyword wouldn't work unless a search term was specified

But browser.js disagrees:

function getShortcutOrURIAndPostData(url, callback = null)
2162     let offset = url.indexOf(" ");
2163     if (offset > 0) {
2164       keyword = url.substr(0, offset);
2165       param = url.substr(offset + 1);
2166     }
2167 
2168     let engine = Services.search.getEngineByAlias(keyword);
2169     if (engine) {

So, I guess for now we should go back to what browser does, cause it's what the users are used to.
taking to complete the "correctness" work started by bug 1172937. plus I basically already wrote the patch in previous comment :)
Assignee: nobody → mak77
Iteration: --- → 42.3 - Aug 10
Rank: 22
Attached patch patch v1Splinter Review
Note, this is the wrong thing to do, since it will use the search alias with an empty string (most engines will cause a redirect), but users are used to this behavior and I don't feel like it's a good time to change a further thing.
Moreover changing just this would make the behavior inconsistent with bookmark keywords.
We could re-evaluate in future.
Attachment #8643758 - Flags: review?(adw)
Comment on attachment 8643758 [details] [diff] [review]
patch v1

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

::: toolkit/components/places/UnifiedComplete.js
@@ +77,5 @@
>  // The hostname will already have been checked for general validity, so we
>  // don't need to be exhaustive here, so allow dashes anywhere.
>  const REGEXP_SINGLEWORD_HOST = new RegExp("^[a-z0-9-]+$", "i");
>  
> +// Regex used to match one or more whitespaces.

whitespaces -> whitespace (no trailing S)
Attachment #8643758 - Flags: review?(adw) → review+
Flags: in-testsuite+
(In reply to Marco Bonardo [::mak] (spotty available until 24 Aug) from comment #5)
> Note, this is the wrong thing to do, since it will use the search alias with
> an empty string (most engines will cause a redirect)
I think it's not "the wrong thing to do", because it is exactly what happens when I focus empty searchbar and press Enter. Is that a wrong action too? :)

Also, well written search plugins don't use empty string in request and there's no redirect:
- When I as a user set "y" as keyword for youtube search, then type "y" or "y " in location bar and then press Enter, I can see the following GET request in browser console: "http://www.youtube.com/"
- When I try "y asdf", request is
"http://www.youtube.com/results?search_query=asdf&page=&utm_source=opensearch"
https://hg.mozilla.org/mozilla-central/rev/c9adece82ecc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
(In reply to arni2033 from comment #8)
> I think it's not "the wrong thing to do", because it is exactly what happens
> when I focus empty searchbar and press Enter. Is that a wrong action too? :)

it's totally different. i think the location bar might do the same where there's nothing typed. Here an alias is typed. btw, for now this discussion doesn't matter.
See Also: → 1192218
Has STR: --- → yes
I have reproduced this bug with Nightly 42.0a1 (2015-07-26) on Windows 7, 64 Bit!

The Bug's fix is verified on Latest Release, Beta, Aurora and Nightly!

Build ID 	20160623154057
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0

Build ID 	20160630123429
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

Build ID 	20160705004031
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0

Build ID 	20160705030222
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Status: RESOLVED → VERIFIED
Status: VERIFIED → RESOLVED
Closed: 4 years ago3 years ago
QA Whiteboard: [bugday-20160706]
Opps, changed the status mistakenly. Reverting it.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.