Omnibox: Don't Promote Intranet Hostnames Pound Sign Space Whatever ("c# foo")

chrome/browser/autocomplete/history_url_provider.cc

Index: chrome/browser/autocomplete/history_url_provider.cc
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index dd2807e4438181216894e244125e7b7dbeb72a3e..fcb72cb818d20d1e4ad74e442c67d4421d8eebe7 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -741,14 +741,41 @@ bool HistoryURLProvider::FixupExactSuggestion(
       break;
   }
 
+  const GURL& url = match->destination_url;
+  const url_parse::Parsed& parsed = url.parsed_for_possibly_invalid_spec();
+  // If the what-you-typed result looks like a single word (which can be
+  // interpreted as an intranet address) followed by a pound sign ("#"),
+  // leave the score for the url-what-you-typed result as is.  The top
+  // result be a search query from SearchProvider. This test fixes cases

[Peter Kasting, 2013/04/03 21:43:49]

Nit: be -> will be, from -> from the?

This sounds maybe inaccurate though, can't we have an inline autocompletion of a
non-exact match outscore both things?

[Mark P, 2013/04/03 22:58:18]

Done.  (x2)
Correct.  Rewrote part of the sentence.

+  // such as "c#" and "c# foo" where the user has visited an intranet
+  // site "c".  We want the search-what-you-typed to beat the

[Peter Kasting, 2013/04/03 21:43:49]

Nit: to beat -> score to beat?

[Mark P, 2013/04/03 22:58:18]

Okay.

+  // URL-what-you-typed in this case.  Most of the below test tries to make

[Peter Kasting, 2013/04/03 21:43:49]

Nit: in -> score in?

[Mark P, 2013/04/03 22:58:18]

Done.

+  // sure that this code does not trigger if the user did anything to
+  // indicate the desired match is a URL.  For instance, "c/# foo" will not
+  // pass the test because those will be classified as input type URL.  The

[Peter Kasting, 2013/04/03 21:43:49]

Nit: those -> that

[Mark P, 2013/04/03 22:58:18]

Done.

+  // parsed.CountCharactersBefore() in the test looks for the presence of a
+  // reference fragment in the URL by checking whether the position differs
+  // included the delimiter (pound sign) versus not including the delimiter.
+  // (One cannot simply check url.ref() because it will not distinguish
+  // between the input "c" and the input "c#", both of which will have empty
+  // reference fragments.)
+  if ((type == UNVISITED_INTRANET) &&
+      (input.type() != AutocompleteInput::URL) &&
+      url.username().empty() && url.password().empty() && url.port().empty() &&
+      (url.path() == "/") && url.query().empty() &&

[Peter Kasting, 2013/04/03 21:43:49]

Nit: Fractionally faster: url.path().length() == 1

[Mark P, 2013/04/03 22:58:18]

A seriously less readable.  It also makes me nervous (for no good reason).  I'd
strongly prefer to leave it as is.

+      (parsed.CountCharactersBefore(url_parse::Parsed::REF, true) !=
+       parsed.CountCharactersBefore(url_parse::Parsed::REF, false))) {
+    return false;
+  }
+
   match->relevance = CalculateRelevance(type, 0);
 
+  // If there are any other matches, then don't promote this match here, in
+  // hopes the caller will be able to inline autocomplete a better suggestion.
+  // DoAutocomplete() will fall back on this match if inline autocompletion
+  // fails.  This matches how we react to never-visited URL inputs in the non-
+  // intranet case.
   if (type == UNVISITED_INTRANET && !matches->empty()) {

[Peter Kasting, 2013/04/03 21:43:49]

Nit: No {}

[Mark P, 2013/04/03 22:58:18]

Done.

-    // If there are any other matches, then don't promote this match here, in
-    // hopes the caller will be able to inline autocomplete a better suggestion.
-    // DoAutocomplete() will fall back on this match if inline autocompletion
-    // fails.  This matches how we react to never-visited URL inputs in the non-
-    // intranet case.
     return false;
   }