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 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;
   }
 
   // Put it on the front of the HistoryMatches for redirect culling.
   CreateOrPromoteMatch(classifier.url_row(), string16::npos, false, matches,
                        true, true);
   return true;
 }
 
 bool HistoryURLProvider::CanFindIntranetURL(
@@ skipping 286 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;
 }