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

chrome/browser/autocomplete/history_url_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/history_url_provider.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 723 matching lines @@
         // this row's data available during the first pass.  That means we
         // either scored it as WHAT_YOU_TYPED or UNVISITED_INTRANET, and to
         // maintain the ordering between passes consistent, we need to score it
         // the same way here.
         type = CanFindIntranetURL(db, input) ?
             UNVISITED_INTRANET : WHAT_YOU_TYPED;
       }
       break;
   }
 
+  const GURL& url = match->destination_url;
+  const url_parse::Parsed& parsed = url.parsed_for_possibly_invalid_spec();
+  if ((type == UNVISITED_INTRANET) &&
+      (input.type() != AutocompleteInput::URL) &&
+      url.username().empty() && url.password().empty() &&
+      (net::RegistryControlledDomainService::GetRegistryLength(url.host(),
+                                                               false) == 0) &&

[Peter Kasting, 2013/04/03 00:21:29]

Why do we have to check the registry length?  Doesn't UNVISITED_INTRANET already
get us this?

[Mark P, 2013/04/03 16:08:01]

You are correct.  Removed.

+      (url.port().empty() || (url.port() == "-1")) &&  // -1 = "default"
+      (url.path().empty() || (url.path() == "/")) &&

[Peter Kasting, 2013/04/03 00:21:29]

For the port and path checks, it'd be nice to avoid checking cases which never
happen.  For example, if the user doesn't specify a port, do we get the empty
port, or -1?  Presumably it's always one and not the other, right?

[Mark P, 2013/04/03 16:08:01]

Done.

+      url.query().empty() &&
+      (parsed.CountCharactersBefore(url_parse::Parsed::REF, true) !=
+       parsed.CountCharactersBefore(url_parse::Parsed::REF, false))) {
+    // If the what-you-typed result looks like a single word (which

[Peter Kasting, 2013/04/03 00:21:29]

Nit: Put this comment above the conditional instead of inside it.  (We should do
this with the comment block just below, too.)

[Mark P, 2013/04/03 16:08:01]

Done.

+    // can be interpreted as an intranet address) followed by a pound
+    // sign ("#"), leave the score for the url-what-you-typed result
+    // as is and hope the top result ends up being a

[Peter Kasting, 2013/04/03 00:21:29]

Nit: This sounds a bit sketchy when you say "hope".  Aren't we guaranteed that
for non-URL input types, if we don't promote the exact match, we'll either get
an inline autocompletion or else some type of search ranking above this?  If so
we don't really need to "hope", we can rely on this.

[Mark P, 2013/04/03 16:08:01]

We're guaranteed to be a search query above this (assuming SearchProvider is
enabled).  Rephrased to eliminate the wishy-washy-ness.

+    // search-what-you-typed.  This test fixes cases such as "c# foo"
+    // where the user has visited an intranet site "c".  We want the
+    // search-what-you-typed to beat the URL-what-you-typed in this
+    // case.  Most of the above test tries to make 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 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.)
+    return false;
+  }
+
   match->relevance = CalculateRelevance(type, 0);
 
   if (type == UNVISITED_INTRANET && !matches->empty()) {
     // 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;
   }
@@ skipping 294 matching lines @@
         &match.contents_class);
   }
   match.description = info.title();
   AutocompleteMatch::ClassifyMatchInString(params->input.text(),
                                            info.title(),
                                            ACMatchClassification::NONE,
                                            &match.description_class);
   RecordAdditionalInfoFromUrlRow(info, &match);
   return match;
 }