Omnibox: Preserve display text and select all on a focus search

components/omnibox/browser/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 "components/omnibox/browser/omnibox_edit_model.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 397 matching lines @@
 
 void OmniboxEditModel::StartAutocomplete(bool has_selected_text,
                                          bool prevent_inline_autocomplete) {
   const base::string16 input_text = MaybePrependKeyword(user_text_);
 
   // Compute the cursor position.  There are three cases:
   //   1. The user is in the midst of typing; there is no inline autocompletion.
   //   2. The user is in the midst of typing; there is inline autocompletion.
   //   3. The user is not in the midst of typing, but is triggering this some
   //      other way, e.g. hitting ctrl-K while the view is showing the permanent
   //      or temporary text.

[Peter Kasting, 2016/10/20 21:42:00]

We need to touch this function.  Because you're changing what the user text is,
the "case 1" code will be triggered more often, when we don't actually want it
to (e.g. all the case 2 cases).  Basically, every time the user hits ctrl-K, we
want to use the "case 2 or 3" code here, so we compute autocomplete suggestions
based on the cursor being at the end of the new text, instead of possibly
somewhere in the middle.

I haven't thought about what the new code or comments need to be.

[Tom (Use chromium acct), 2016/10/25 02:11:08]

I couldn't really figure out a way to make this work, so I just decided to make
the callers responsible for handling the caret position.  I think this is better
than StartAutocomplete trying to figure out the caret position based on various
hints.

   size_t cursor_position;
   if (!has_temporary_text_ && (user_text_ == view_->GetText())) {
     // Case 1 above.  In this case there's a meaningful current cursor position,
     // so we read it from the view.  (Note that if there is a selected range,
     // the "cursor position" is considered to be the selection's end.)
     size_t start;
     view_->GetSelectionBounds(&start, &cursor_position);
 
     // For keyword searches, the text that AutocompleteInput expects is of the
     // form "<keyword> <query>", where our query is |user_text_|.  So we need to
@@ skipping 124 matching lines @@
 
   DCHECK(popup_model());
   view_->OpenMatch(match, disposition, alternate_nav_url, base::string16(),
                    popup_model()->selected_line());
 }
 
 void OmniboxEditModel::EnterKeywordModeForDefaultSearchProvider(
     KeywordModeEntryMethod entry_method) {
   autocomplete_controller()->Stop(false);
 
+  user_text_ = user_input_in_progress_ ? view_->GetText() : base::string16();
   user_input_in_progress_ = true;

[Tom (Use chromium acct), 2016/10/20 20:34:54]

Should this be SetInputInProgress(true)?

[Peter Kasting, 2016/10/20 21:42:00]

Yes, I think so.

You should consider using SetUserText() in place of these two lines.  This would
also allow deleting "is_keyword_hint_ = false;" below.  I would double-check
that that doesn't do anything you explicitly don't want.

   keyword_ = client_->GetTemplateURLService()->
       GetDefaultSearchProvider()->keyword();
   is_keyword_hint_ = false;
   keyword_mode_entry_method_ = entry_method;
 
   StartAutocomplete(false, false);
 
   UMA_HISTOGRAM_ENUMERATION(
       kEnteredKeywordModeHistogram, static_cast<int>(entry_method),
       static_cast<int>(KeywordModeEntryMethod::NUM_ITEMS));
@@ skipping 857 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();
 
   client_->OnFocusChanged(focus_state_, reason);
 }