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

chrome/browser/autocomplete/autocomplete_edit_view_gtk.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_gtk.h"
 
 #include <gdk/gdkkeysyms.h>
 #include <gtk/gtk.h>
 
 #include <algorithm>
@@ skipping 761 matching lines @@
       ) {
     instant_animation_->set_delegate(this);
     instant_animation_->Start();
   }
 
   gtk_widget_show(instant_view_);
   AdjustVerticalAlignmentOfInstantView();
   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_),
                                            GTK_TEXT_WINDOW_LEFT) +
       gtk_text_view_get_border_window_size(GTK_TEXT_VIEW(text_view_),
                                            GTK_TEXT_WINDOW_RIGHT) +
       gtk_text_view_get_left_margin(GTK_TEXT_VIEW(text_view_)) +
       gtk_text_view_get_right_margin(GTK_TEXT_VIEW(text_view_));
 
   GtkTextIter start, end;
@@ skipping 35 matching lines @@
 
 #if defined(TOOLKIT_VIEWS)
 views::View* AutocompleteEditViewGtk::AddToView(views::View* parent) {
   views::NativeViewHost* host = new views::NativeViewHost;
   parent->AddChildView(host);
   host->set_focus_view(parent);
   host->Attach(GetNativeView());
   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()));
   accessible_widget_helper_->SetWidgetName(
       text_view(), l10n_util::GetStringUTF8(IDS_ACCNAME_LOCATION));
 }
 
 // static
 AutocompleteEditView* AutocompleteEditViewGtk::Create(
     AutocompleteEditController* controller,
@@ skipping 38 matching lines @@
 
 void AutocompleteEditViewGtk::Observe(NotificationType type,
                                       const NotificationSource& source,
                                       const NotificationDetails& details) {
   DCHECK(type == NotificationType::BROWSER_THEME_CHANGED);
 
   SetBaseColor();
 }
 
 void AutocompleteEditViewGtk::AnimationEnded(const ui::Animation* animation) {
-  controller_->OnCommitSuggestedText(GetText());
+  controller_->OnCommitSuggestedText(false);
 }
 
 void AutocompleteEditViewGtk::AnimationProgressed(
     const ui::Animation* animation) {
   UpdateInstantViewColors();
 }
 
 void AutocompleteEditViewGtk::AnimationCanceled(
     const ui::Animation* animation) {
   UpdateInstantViewColors();
@@ skipping 425 matching lines @@
     GtkMovementStep step,
     gint count,
     gboolean extend_selection) {
   GtkTextIter sel_start, sel_end;
   gboolean has_selection =
       gtk_text_buffer_get_selection_bounds(text_buffer_, &sel_start, &sel_end);
   bool handled = false;
 
   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
     // input caret, it's much simpler and good enough to check whole content's
     // direction.
     PangoDirection content_dir = GetContentDirection();
     gint count_towards_end = content_dir == PANGO_DIRECTION_RTL ? -1 : 1;
 
     // We want the GtkEntry behavior when you move the cursor while you have a
     // selection.  GtkTextView just drops the selection and moves the cursor,
     // but instead we want to move the cursor to the appropiate end of the
     // selection.
     if (has_selection) {
       // We have a selection and start / end are in ascending order.
       // Cursor placement will remove the selection, so we need inform
       // |model_| about this change by
       // calling On{Before|After}PossibleChange() methods.
       OnBeforePossibleChange();
       gtk_text_buffer_place_cursor(
           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).
     model_->OnUpOrDownKeyPressed(model_->result().size() * count);
     handled = true;
   } else if (step == GTK_MOVEMENT_DISPLAY_LINES) {  // Arrow up and down.
     model_->OnUpOrDownKeyPressed(count);
     handled = true;
   }
 
@@ skipping 247 matching lines @@
 void AutocompleteEditViewGtk::HandleViewMoveFocus(GtkWidget* widget,
                                                   GtkDirectionType direction) {
   if (!tab_was_pressed_)
     return;
 
   // If special behavior is triggered, then stop the signal emission to
   // prevent the focus from being moved.
   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 {
+#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);
   }
 }
 
 void AutocompleteEditViewGtk::HandleCopyClipboard(GtkWidget* sender) {
   HandleCopyOrCutClipboard(true);
 }
@@ skipping 205 matching lines @@
   gtk_text_buffer_get_iter_at_mark(text_buffer_, &end, instant_mark_);
 #if GTK_CHECK_VERSION(2, 20, 0)
   // We need to count the length of the text being composed, because we treat
   // it as part of the content in GetText().
   return gtk_text_iter_get_offset(&end) + preedit_.size();
 #else
   return gtk_text_iter_get_offset(&end);
 #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
   // being composed by the input method) is not empty, which is not treated as
   // a part of the text content inside GtkTextView. And it's ok to simply return
   // in this case, as this method will be called again when the preedit string
   // gets committed.
   if (preedit_.size()) {
     strikethrough_ = CharRange();
     return;
@@ skipping 55 matching lines @@
   }
 }
 
 void AutocompleteEditViewGtk::StopAnimation() {
   // Clear the animation delegate so we don't get an AnimationEnded() callback.
   instant_animation_->set_delegate(NULL);
   instant_animation_->Stop();
   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();
 }
 
 void AutocompleteEditViewGtk::SavePrimarySelection(
     const std::string& selected_text) {
   GtkClipboard* clipboard =
       gtk_widget_get_clipboard(text_view_, GDK_SELECTION_PRIMARY);
   DCHECK(clipboard);
@@ skipping 248 matching lines @@
   // baseline, so we need to move the |instant_view_| down to make sure it
   // has the same baseline as the |text_view_|.
   PangoLayout* layout = gtk_label_get_layout(GTK_LABEL(instant_view_));
   int height;
   pango_layout_get_size(layout, NULL, &height);
   PangoLayoutIter* iter = pango_layout_get_iter(layout);
   int baseline = pango_layout_iter_get_baseline(iter);
   pango_layout_iter_free(iter);
   g_object_set(instant_anchor_tag_, "rise", baseline - height, NULL);
 }