Allow entering keyword mode with space when there's inline autocompletion.

chrome/browser/ui/omnibox/omnibox_edit_model.cc

 // Copyright 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/ui/omnibox/omnibox_edit_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 884 matching lines @@
   DCHECK(is_keyword_hint_ && !keyword_.empty());
 
   autocomplete_controller()->Stop(false);
   is_keyword_hint_ = false;
 
   if (popup_model() && popup_model()->IsOpen())
     popup_model()->SetSelectedLineState(OmniboxPopupModel::KEYWORD);
   else
     StartAutocomplete(false, true, true);
 
-  // Ensure the current selection is saved before showing keyword mode
-  // so that moving to another line and then reverting the text will restore
-  // the current state properly.
-  bool save_original_selection = !has_temporary_text_;
-  has_temporary_text_ = true;
-  const AutocompleteMatch& match = CurrentMatch(NULL);
-  original_url_ = match.destination_url;
-  view_->OnTemporaryTextMaybeChanged(
-      DisplayTextFromUserText(match.fill_into_edit),
-      save_original_selection, true);
+  // When entering keyword mode via tab, the new text to show is whatever the
+  // newly-selected match in the dropdown is.  When entering via space, however,
+  // we should make sure to use the actual |user_text_| as the basis for the new
+  // text.  This ensures that if the user types "<keyword><space>" and the
+  // default match would have inline autocompleted a further string (e.g.
+  // because there's a past multi-word search beginning with this keyword), the
+  // inline autocompletion doesn't get filled in as the keyword search query
+  // text.
+  //
+  // We also treat tabbing into keyword mode like tabbing through the popup in
+  // that we set |has_temporary_text_|, whereas pressing space is treated like
+  // a new keystroke that changes the current match instead of overlaying it
+  // with a temporary one.  This is important because rerunning autocomplete
+  // after the user pressed space, which will have happened just before reaching
+  // here, may have generated a new match, which the user won't actually see and
+  // which we don't want to switch back to when existing keyword mode; see
+  // comments in ClearKeyword().
+  if (entered_method == ENTERED_KEYWORD_MODE_VIA_TAB) {
+    // Ensure the current selection is saved before showing keyword mode
+    // so that moving to another line and then reverting the text will restore
+    // the current state properly.
+    bool save_original_selection = !has_temporary_text_;
+    has_temporary_text_ = true;
+    const AutocompleteMatch& match = CurrentMatch(NULL);
+    original_url_ = match.destination_url;
+    view_->OnTemporaryTextMaybeChanged(
+        DisplayTextFromUserText(match.fill_into_edit),
+        save_original_selection, true);
+  } else {
+    view_->OnTemporaryTextMaybeChanged(DisplayTextFromUserText(user_text_),
+                                       false, true);
+  }
 
   view_->UpdatePlaceholderText();
 
   content::RecordAction(base::UserMetricsAction("AcceptedKeywordHint"));
   UMA_HISTOGRAM_ENUMERATION(kEnteredKeywordModeHistogram, entered_method,
                             ENTERED_KEYWORD_MODE_NUM_ITEMS);
 
   return true;
 }
 
 void OmniboxEditModel::AcceptTemporaryTextAsUserText() {
   InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
   has_temporary_text_ = false;
 
   if (user_input_in_progress_ || !in_revert_)
     delegate_->OnInputStateChanged();
 }
 
 void OmniboxEditModel::ClearKeyword() {
   autocomplete_controller()->Stop(false);
   omnibox_controller_->ClearPopupKeywordMode();
 
   const base::string16 window_text(keyword_ + view_->GetText());
 
-  // Only reset the result if the edit text has changed since the
-  // keyword was accepted, or if the popup is closed.
-  if (!just_deleted_text_ && view_->GetText().empty() && popup_model() &&
-      popup_model()->IsOpen()) {
+  // If we've tabbed into keyword mode and haven't done anything else,
+  // |has_temporary_text_| will be true, and we should just revert into keyword
+  // hint mode; otherwise we do a more complete state update/revert via
+  // OnBefore/AfterPossibleChange().
+  //
+  // If we were to do the "complete state revert" all the time, then in cases
+  // where our associated keyword match is in the middle of the popup instead of
+  // on the first line, tabbing into it and then hitting shift-tab would reset
+  // the entire popup contents, and we'd wind up with the first line selected.
+  //
+  // If we were instead to "just switch back to keyword hint mode" all the time,
+  // we could wind up with strange state in some cases.  For example, if a user
+  // has a keyword named "x", an inline-autocompletable history site "xyz.com",
+  // and a lower-ranked inline-autocompletable search "x y", then typing "x"
+  // will inline autocomplete to "xyz.com", hitting space will toggle into
+  // keyword mode, but then hitting backspace might wind up with the default
+  // match as the "x y" search, due to the particulars of how we re-run
+  // autocomplete for "x " before toggling into keyword mode to begin with.
+  if (has_temporary_text_) {
     is_keyword_hint_ = true;
     view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length(),
         false, true);
   } else {
     view_->OnBeforePossibleChange();
     view_->SetWindowTextAndCaretPos(window_text.c_str(), keyword_.length(),
         false, false);
     keyword_.clear();
     is_keyword_hint_ = false;
     view_->OnAfterPossibleChange();
@@ skipping 452 matching lines @@
 
   if (revert_popup && popup_model())
     popup_model()->ResetToDefaultMatch();
   view_->OnRevertTemporaryText();
 }
 
 bool OmniboxEditModel::MaybeAcceptKeywordBySpace(
     const base::string16& new_text) {
   size_t keyword_length = new_text.length() - 1;
   return (paste_state_ == NONE) && is_keyword_hint_ &&
-      inline_autocomplete_text_.empty() &&
       (keyword_.length() == keyword_length) &&
       IsSpaceCharForAcceptingKeyword(new_text[keyword_length]) &&
       !new_text.compare(0, keyword_length, keyword_, 0, keyword_length) &&
       AcceptKeyword(ENTERED_KEYWORD_MODE_VIA_SPACE_AT_END);
 }
 
 bool OmniboxEditModel::CreatedKeywordSearchByInsertingSpaceInMiddle(
     const base::string16& old_text,
     const base::string16& new_text,
     size_t caret_position) const {
@@ skipping 77 matching lines @@
   // Update state and notify view if the omnibox has focus and the caret
   // visibility changed.
   const bool was_caret_visible = is_caret_visible();
   focus_state_ = state;
   if (focus_state_ != OMNIBOX_FOCUS_NONE &&
       is_caret_visible() != was_caret_visible)
     view_->ApplyCaretVisibility();
 
   delegate_->OnFocusChanged(focus_state_, reason);
 }