Cursor jumps to end of omnibox when activating keyword mode via inserted space

chrome/browser/ui/omnibox/omnibox_edit_model.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/ui/omnibox/omnibox_edit_model.h"
 
 #include <string>
 
 #include "base/format_macros.h"
 #include "base/metrics/histogram.h"
@@ skipping 865 matching lines @@
       // pressed, even though maybe it isn't any more.  There is no obvious
       // right answer here :(
     }
     view_->OnTemporaryTextMaybeChanged(DisplayTextFromUserText(text),
                                        save_original_selection);
     return;
   }
 
   bool call_controller_onchanged = true;
   inline_autocomplete_text_ = text;
-  if (view_->OnInlineAutocompleteTextMaybeChanged(
-      DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
-      DisplayTextFromUserText(user_text_).length()))
+  if (KeywordIsSelected() &&
+      view_->GetText() != DisplayTextFromUserText(user_text_)) {

[Peter Kasting, 2012/10/02 07:37:12]

Does this do the right thing in cases where we have (new or existing) inline
autocompletion in keyword mode?  Or when we're tabbing through the dropdown and
thus have temporary text?  I'm worried about those causing cases where the
display text does not match the user text and thus erroneously triggering this
block.

[Joe Thomas, 2012/10/02 18:39:23]

When keyword search mode is activated, AFAIK there is no more inline
autocompletion. OmniboxEditModel::OnPopupDataChanged is always called with  NULL
value for 'text' parameter while 'keyword' search mode is active. And this sets
the existing inline_autocomplete_text_ to NULL in
OmniboxEditModel::OnPopupDataChanged.
This case is handled in the block just above and it returns from there.

[Peter Kasting, 2012/10/02 18:42:35]

We can definitely inline autocomplete while in keyword search mode.  I just
tested now: tab-to-search google.com and type "test"; then after the page loads,
do the same thing, and "test" will inline autocomplete as soon as you type the
't'.

[Joe Thomas, 2012/10/02 18:57:24]

Yes, you are right. The patch breaks that usecase. I will re-work on the patch.
I did not know that there is an inline autocompletion available for search
strings. Sorry for my ignorance.

+    view_->SetWindowTextAndCaretPos(DisplayTextFromUserText(user_text_), 0,
+        false, true);
     call_controller_onchanged = false;
+  } else if (view_->OnInlineAutocompleteTextMaybeChanged(
+                DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
+                DisplayTextFromUserText(user_text_).length())) {
+    call_controller_onchanged = false;
+  }
 
   // If |has_temporary_text_| is true, then we previously had a manual selection
   // but now don't (or |destination_for_temporary_text_change| would have been
   // non-NULL). This can happen when deleting the selected item in the popup.
   // In this case, we've already reverted the popup to the default match, so we
   // need to revert ourselves as well.
   if (has_temporary_text_) {
     RevertTemporaryText(false);
     call_controller_onchanged = false;
   }
@@ skipping 365 matching lines @@
 }
 
 void OmniboxEditModel::ClassifyStringForPasteAndGo(
     const string16& text,
     AutocompleteMatch* match,
     GURL* alternate_nav_url) const {
   DCHECK(match);
   AutocompleteClassifierFactory::GetForProfile(profile_)->Classify(text,
       string16(), false, false, match, alternate_nav_url);
 }