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

chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc

Index: chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
index 4787db599ea7bd7c937622d46e555108fbe2e7f4..e8d1edec1c92a201a77a36fe7503e58c99bcef4b 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
@@ -779,6 +779,11 @@ void AutocompleteEditViewGtk::SetInstantSuggestion(const string16& suggestion) {
   UpdateInstantViewColors();
 }
 
+string16 AutocompleteEditViewGtk::GetInstantSuggestion() const {
+  const gchar* suggestion = gtk_label_get_text(GTK_LABEL(instant_view_));
+  return suggestion ? UTF8ToUTF16(suggestion) : string16();
+}
+
 int AutocompleteEditViewGtk::TextWidth() const {
   int horizontal_border_size =
       gtk_text_view_get_border_window_size(GTK_TEXT_VIEW(text_view_),
@@ -834,12 +839,6 @@ views::View* AutocompleteEditViewGtk::AddToView(views::View* parent) {
   return host;
 }
 
-bool AutocompleteEditViewGtk::CommitInstantSuggestion(
-    const string16& typed_text,
-    const string16& suggestion) {
-  return CommitInstantSuggestion();
-}
-
 void AutocompleteEditViewGtk::EnableAccessibility() {
   accessible_widget_helper_.reset(
       new AccessibleWidgetHelper(text_view(), model_->profile()));
@@ -898,7 +897,7 @@ void AutocompleteEditViewGtk::Observe(NotificationType type,
 }
 
 void AutocompleteEditViewGtk::AnimationEnded(const ui::Animation* animation) {
-  controller_->OnCommitSuggestedText(GetText());
+  controller_->OnCommitSuggestedText(false);
 }
 
 void AutocompleteEditViewGtk::AnimationProgressed(
@@ -1344,9 +1343,6 @@ void AutocompleteEditViewGtk::HandleViewMoveCursor(
 
   if (step == GTK_MOVEMENT_VISUAL_POSITIONS && !extend_selection &&
       (count == 1 || count == -1)) {
-    gint cursor_pos;
-    g_object_get(G_OBJECT(text_buffer_), "cursor-position", &cursor_pos, NULL);
-
     // We need to take the content direction into account when handling cursor
     // movement, because the behavior of Left and Right key will be inverted if
     // the direction is RTL. Although we should check the direction around the
@@ -1369,8 +1365,9 @@ void AutocompleteEditViewGtk::HandleViewMoveCursor(
           text_buffer_, count == count_towards_end ? &sel_end : &sel_start);
       OnAfterPossibleChange();
       handled = true;
-    } else if (count == count_towards_end && cursor_pos == GetTextLength()) {
-      handled = controller_->OnCommitSuggestedText(GetText());
+    } else if (count == count_towards_end &&
+               GetCursorPosition() == GetTextLength()) {
+      handled = controller_->OnCommitSuggestedText(true);
     }
   } else if (step == GTK_MOVEMENT_PAGES) {  // Page up and down.
     // Multiply by count for the direction (if we move too much that's ok).
@@ -1638,15 +1635,31 @@ void AutocompleteEditViewGtk::HandleViewMoveFocus(GtkWidget* widget,
   bool handled = false;
 
   // Trigger Tab to search behavior only when Tab key is pressed.
-  if (model_->is_keyword_hint()) {
+  if (model_->is_keyword_hint())
     handled = model_->AcceptKeyword();
-  } else if (GTK_WIDGET_VISIBLE(instant_view_)) {
-    controller_->OnCommitSuggestedText(GetText());
+
+#if GTK_CHECK_VERSION(2, 20, 0)
+  if (!handled && !preedit_.empty())
     handled = true;
-  } else {
-    handled = controller_->AcceptCurrentInstantPreview();
+#endif
+
+  if (!handled && GTK_WIDGET_VISIBLE(instant_view_))
+    handled = controller_->OnCommitSuggestedText(true);
+
+  if (!handled) {
+    CharRange selection = GetSelection();
+    int length = GetTextLength();
+    if (selection.cp_min != selection.cp_max || selection.cp_min < length) {
+      OnBeforePossibleChange();

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

Do we need the OnBefore/after here?

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

Yes we need it to inform the model possible selection range change. It may cause
the model to accept the inline autocomplete and update popup accordingly. Just
like what Right key does.

+      SetCursorPosition(length);
+      OnAfterPossibleChange();
+      handled = true;
+    }
   }
 
+  if (!handled)
+    handled = controller_->AcceptCurrentInstantPreview();
+
   if (handled) {
     static guint signal_id = g_signal_lookup("move-focus", GTK_TYPE_WIDGET);
     g_signal_stop_emission(widget, signal_id, 0);
@@ -1871,6 +1884,18 @@ int AutocompleteEditViewGtk::GetTextLength() const {
 #endif
 }
 
+int AutocompleteEditViewGtk::GetCursorPosition() const {
+  gint cursor_pos;

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

Can this be implemented in terms of GetSelection?

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

I'll recheck this part to see how we can do it better. After migrating to
string16, the code may have problem handling non-BMP characters.

+  g_object_get(G_OBJECT(text_buffer_), "cursor-position", &cursor_pos, NULL);
+  return cursor_pos;
+}
+
+void AutocompleteEditViewGtk::SetCursorPosition(int cursor_position) {
+  GtkTextIter cursor;
+  gtk_text_buffer_get_iter_at_offset(text_buffer_, &cursor, cursor_position);
+  gtk_text_buffer_place_cursor(text_buffer_, &cursor);
+}
+
 void AutocompleteEditViewGtk::EmphasizeURLComponents() {
 #if GTK_CHECK_VERSION(2, 20, 0)
   // We can't change the text style easily, if the preedit string (the text
@@ -1946,16 +1971,6 @@ void AutocompleteEditViewGtk::StopAnimation() {
   UpdateInstantViewColors();
 }
 
-bool AutocompleteEditViewGtk::CommitInstantSuggestion() {
-  const gchar* suggestion = gtk_label_get_text(GTK_LABEL(instant_view_));
-  if (!suggestion || !*suggestion)
-    return false;
-
-  model()->FinalizeInstantQuery(GetText(),
-                                UTF8ToUTF16(suggestion));
-  return true;
-}
-
 void AutocompleteEditViewGtk::TextChanged() {
   EmphasizeURLComponents();
   controller_->OnChanged();