Hitting Tab should always move cursor to end of omnibox text.

chrome/browser/autocomplete/autocomplete_edit_view_win.cc

 // Copyright (c) 2011 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/autocomplete_edit_view_win.h"
 
 #include <algorithm>
 #include <locale>
 #include <string>
 
@@ skipping 906 matching lines @@
 
 CommandUpdater* AutocompleteEditViewWin::GetCommandUpdater() {
   return command_updater_;
 }
 
 void AutocompleteEditViewWin::SetInstantSuggestion(const string16& suggestion) {
   // On Windows, we shows the suggestion in LocationBarView.
   NOTREACHED();
 }
 
+string16 AutocompleteEditViewWin::GetInstantSuggestion() const {
+  // On Windows, we shows the suggestion in LocationBarView.
+  NOTREACHED();
+  return string16();
+}
+
 int AutocompleteEditViewWin::TextWidth() const {
   return WidthNeededToDisplay(GetText());
 }
 
 bool AutocompleteEditViewWin::IsImeComposing() const {
   bool ime_composing = false;
   HIMC context = ImmGetContext(m_hWnd);
   if (context) {
     ime_composing = !!ImmGetCompositionString(context, GCS_COMPSTR, NULL, 0);
     ImmReleaseContext(m_hWnd, context);
   }
   return ime_composing;
 }
 
 views::View* AutocompleteEditViewWin::AddToView(views::View* parent) {
   views::NativeViewHost* host = new views::NativeViewHost;
   parent->AddChildView(host);
   host->set_focus_view(parent);
   host->Attach(GetNativeView());
   return host;
 }
 
-bool AutocompleteEditViewWin::CommitInstantSuggestion(
-    const string16& typed_text,
-    const string16& suggested_text) {
-  model_->FinalizeInstantQuery(typed_text, suggested_text);
-  return true;
-}
-
 void AutocompleteEditViewWin::PasteAndGo(const string16& text) {
   if (CanPasteAndGo(text))
     model_->PasteAndGo();
 }
 
 bool AutocompleteEditViewWin::SkipDefaultKeyEventProcessing(
     const views::KeyEvent& e) {
   ui::KeyboardCode key = e.GetKeyCode();
   // We don't process ALT + numpad digit as accelerators, they are used for
   // entering special characters.  We do translate alt-home.
@@ skipping 875 matching lines @@
   switch (key) {
     case VK_RIGHT:
       // TODO(sky): figure out RTL.
       if (base::i18n::IsRTL())
         return false;
       {
         CHARRANGE selection;
         GetSel(selection);
         return (selection.cpMin == selection.cpMax) &&
             (selection.cpMin == GetTextLength()) &&
-            controller_->OnCommitSuggestedText(GetText());
+            controller_->OnCommitSuggestedText(true);
       }
 
     case VK_RETURN:
       model_->AcceptInput((flags & KF_ALTDOWN) ?
           NEW_FOREGROUND_TAB : CURRENT_TAB, false);
       return true;
 
     case VK_PRIOR:
     case VK_NEXT:
       count = model_->result().size();
@@ skipping 108 matching lines @@
       ScopedFreeze freeze(this, GetTextObjectModel());
       model_->ClearKeyword(GetText());
       return true;
     }
 
     case VK_TAB: {
       if (model_->is_keyword_hint()) {
         // Accept the keyword.
         ScopedFreeze freeze(this, GetTextObjectModel());
         model_->AcceptKeyword();
+      } else if (!IsCaretAtEnd()) {
+        ScopedFreeze freeze(this, GetTextObjectModel());

[sky, 2011/01/27 19:55:31]

Again, do you need onbefore/after here?

+        OnBeforePossibleChange();
+        PlaceCaretAt(GetTextLength());
+        OnAfterPossibleChange();
       } else {
-        controller_->OnCommitSuggestedText(GetText());
+        controller_->OnCommitSuggestedText(true);
       }
       return true;
     }
 
     case 0xbb:  // Ctrl-'='.  Triggers subscripting (even in plain text mode).
                 // We don't use VK_OEM_PLUS in case the macro isn't defined.
                 // (e.g., we don't have this symbol in embeded environment).
       return true;
 
     default:
@@ skipping 569 matching lines @@
 }
 
 int AutocompleteEditViewWin::WidthNeededToDisplay(
     const string16& text) const {
   // Use font_.GetStringWidth() instead of
   // PosFromChar(location_entry_->GetTextLength()) because PosFromChar() is
   // apparently buggy. In both LTR UI and RTL UI with left-to-right layout,
   // PosFromChar(i) might return 0 when i is greater than 1.
   return font_.GetStringWidth(text) + GetHorizontalMargin();
 }
+
+bool AutocompleteEditViewWin::IsCaretAtEnd() const {
+  long length = GetTextLength();

[sky, 2011/01/27 19:55:31]

For consistency it would be nice if you had this on all platforms.

[James Su, 2011/01/27 20:07:16]

This is a method of CRichEditCtrl, we probably need to use a different name if
we want to add it into AutocompleteEditView interface. Do you have any
suggestion?

[sky, 2011/01/27 21:47:20]

I wasn't suggesting to make it part of the interface, just that you make the
implementation on the other platforms similiar using the same name. For example,
on the mac you have:

NSRange selected_range = GetSelectedRange();
  825     NSUInteger length = [[[field_ currentEditor] string] length];
  826     if (selected_range.length != 0 || selected_range.location < length) {


Which could be the same as if (!IsCaretAtEnd).

[James Su, 2011/01/27 22:04:59]

Oh I see. How about add IsCaretAtEnd and PlaceCaretAt to the interface?

+  CHARRANGE sel;
+  GetSelection(sel);
+  return sel.cpMin == sel.cpMax && sel.cpMin == length;
+}