diff options
author | suzhe@google.com <suzhe@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-13 18:02:45 +0000 |
---|---|---|
committer | suzhe@google.com <suzhe@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-13 18:02:45 +0000 |
commit | b9fc810b6adcc42e4d72cac03482c515696e2d96 (patch) | |
tree | 0c5fd57bd37440acffbce02dbec6fc0c365a8402 | |
parent | d4aaa95e6481e57663544b63109f73906c0e7d61 (diff) | |
download | chromium_src-b9fc810b6adcc42e4d72cac03482c515696e2d96.zip chromium_src-b9fc810b6adcc42e4d72cac03482c515696e2d96.tar.gz chromium_src-b9fc810b6adcc42e4d72cac03482c515696e2d96.tar.bz2 |
Fix Instant suggest issues in Linux Views port.
This CL changes Linux Views port to use the same code of Linux Gtk port
for displaying Instant suggest rather than using the code of Windows
port. The reason is: GtkTextView used by AutocompleteEditViewGtk can't
work with SuggestedTextView very well.
This CL also fixes some issues in AutocompleteEditViewGtk related to RTL
text.
BUG=66167
BUG=66151
TEST=See issue report.
Review URL: http://codereview.chromium.org/5698006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69017 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc | 102 | ||||
-rw-r--r-- | chrome/browser/ui/views/location_bar/location_bar_view.cc | 30 | ||||
-rw-r--r-- | chrome/browser/ui/views/location_bar/location_bar_view.h | 15 | ||||
-rw-r--r-- | chrome/chrome_browser.gypi | 2 |
4 files changed, 103 insertions, 46 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc index f020e2e4..ca70f1b 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc @@ -433,10 +433,22 @@ int AutocompleteEditViewGtk::TextWidth() { &start, &first_char_bounds); gtk_text_view_get_iter_location(GTK_TEXT_VIEW(text_view_), &end, &last_char_bounds); - return ((last_char_bounds.x > first_char_bounds.x) ? - (last_char_bounds.x + last_char_bounds.width - first_char_bounds.x) : - (first_char_bounds.x - last_char_bounds.x + last_char_bounds.width)) + - horizontal_border_size; + + gint first_char_start = first_char_bounds.x; + gint first_char_end = first_char_start + first_char_bounds.width; + gint last_char_start = last_char_bounds.x; + gint last_char_end = last_char_start + last_char_bounds.width; + + // bounds width could be negative for RTL text. + if (first_char_start > first_char_end) + std::swap(first_char_start, first_char_end); + if (last_char_start > last_char_end) + std::swap(last_char_start, last_char_end); + + gint text_width = first_char_start < last_char_start ? + last_char_end - first_char_start : first_char_end - last_char_start; + + return text_width + horizontal_border_size; } int AutocompleteEditViewGtk::WidthOfTextAfterCursor() { @@ -848,12 +860,17 @@ void AutocompleteEditViewGtk::SetBaseColor() { } void AutocompleteEditViewGtk::UpdateInstantViewColors() { -#if !defined(TOOLKIT_VIEWS) SkColor selection_text, selection_bg; GdkColor faded_text, normal_bg; - if (theme_provider_->UseGtkTheme()) { - GtkStyle* style = gtk_rc_get_style(text_view_); +#if defined(TOOLKIT_VIEWS) + bool use_gtk = false; +#else + bool use_gtk = theme_provider_->UseGtkTheme(); +#endif + + if (use_gtk) { + GtkStyle* style = gtk_rc_get_style(instant_view_); faded_text = gtk_util::AverageColors( style->text[GTK_STATE_NORMAL], style->base[GTK_STATE_NORMAL]); @@ -863,12 +880,23 @@ void AutocompleteEditViewGtk::UpdateInstantViewColors() { selection_bg = gfx::GdkColorToSkColor(style->base[GTK_STATE_SELECTED]); } else { gdk_color_parse(kTextBaseColor, &faded_text); - normal_bg = LocationBarViewGtk::kBackgroundColor; +#if defined(TOOLKIT_VIEWS) + normal_bg = gfx::SkColorToGdkColor( + LocationBarView::GetColor(ToolbarModel::NONE, + LocationBarView::BACKGROUND)); + selection_text = LocationBarView::GetColor( + ToolbarModel::NONE, LocationBarView::SELECTED_TEXT); + + GtkStyle* style = gtk_rc_get_style(instant_view_); + selection_bg = gfx::GdkColorToSkColor(style->base[GTK_STATE_SELECTED]); +#else + normal_bg = LocationBarViewGtk::kBackgroundColor; selection_text = theme_provider_->get_active_selection_fg_color(); selection_bg = theme_provider_->get_active_selection_bg_color(); +#endif } double alpha = instant_animation_->is_animating() ? @@ -892,9 +920,6 @@ void AutocompleteEditViewGtk::UpdateInstantViewColors() { // is NORMAL, and the background is transparent. gtk_widget_modify_fg(instant_view_, GTK_STATE_NORMAL, &text); } -#else // defined(TOOLKIT_VIEWS) - // We don't worry about views because it doesn't use the instant view. -#endif } void AutocompleteEditViewGtk::HandleBeginUserAction(GtkTextBuffer* sender) { @@ -1172,39 +1197,45 @@ void AutocompleteEditViewGtk::HandleViewMoveCursor( GtkTextIter sel_start, sel_end; gboolean has_selection = gtk_text_buffer_get_selection_bounds(text_buffer_, &sel_start, &sel_end); + bool handled = false; - bool handled = true; - - // 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 (step == GTK_MOVEMENT_VISUAL_POSITIONS && !extend_selection) { - if ((count == 1 || count == -1) && has_selection) { + 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. + // 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 == 1 ? &sel_end : &sel_start); + gtk_text_buffer_place_cursor( + text_buffer_, count == count_towards_end ? &sel_end : &sel_start); OnAfterPossibleChange(); - } else if (count == 1 && !has_selection) { - gint cursor_pos; - g_object_get(G_OBJECT(text_buffer_), "cursor-position", &cursor_pos, - NULL); - if (cursor_pos == GetTextLength()) - controller_->OnCommitSuggestedText(GetText()); - else - handled = false; - } else { - handled = false; + handled = true; + } else if (count == count_towards_end && cursor_pos == GetTextLength()) { + handled = controller_->OnCommitSuggestedText(GetText()); } } 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); - } else { - handled = false; + handled = true; } if (!handled) { @@ -1470,9 +1501,6 @@ void AutocompleteEditViewGtk::HandleViewMoveFocus(GtkWidget* widget, handled = true; } } else { - // TODO(estade): this only works for linux/gtk; linux/views doesn't use - // |instant_view_| so its visibility is not an indicator of whether we - // have a suggestion. if (GTK_WIDGET_VISIBLE(instant_view_)) { controller_->OnCommitSuggestedText(GetText()); handled = true; diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc index 4e9cdcf..1babc92 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -27,7 +27,6 @@ #include "chrome/browser/search_engines/template_url_model.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/browser/ui/view_ids.h" -#include "chrome/browser/ui/views/location_bar/suggested_text_view.h" #include "chrome/browser/ui/views/browser_dialogs.h" #include "chrome/browser/ui/views/location_bar/content_setting_image_view.h" #include "chrome/browser/ui/views/location_bar/ev_bubble_view.h" @@ -46,6 +45,7 @@ #include "views/drag_utils.h" #if defined(OS_WIN) +#include "chrome/browser/ui/views/location_bar/suggested_text_view.h" #include "chrome/browser/views/first_run_bubble.h" #endif @@ -103,7 +103,9 @@ LocationBarView::LocationBarView(Profile* profile, ev_bubble_view_(NULL), location_entry_view_(NULL), selected_keyword_view_(NULL), +#if defined(OS_WIN) suggested_text_view_(NULL), +#endif keyword_hint_view_(NULL), star_view_(NULL), mode_(mode), @@ -408,12 +410,14 @@ gfx::Point LocationBarView::GetLocationEntryOrigin() const { return origin; } +#if defined(OS_WIN) void LocationBarView::OnCommitSuggestedText() { InstantController* instant = delegate_->GetInstant(); DCHECK(instant); DCHECK(suggested_text_view_); OnCommitSuggestedText(location_entry_->GetText()); } +#endif gfx::Size LocationBarView::GetPreferredSize() { return gfx::Size(0, GetThemeProvider()->GetBitmapNamed(mode_ == POPUP ? @@ -613,6 +617,7 @@ void LocationBarView::Layout() { } } +#if defined(OS_WIN) // Layout out the suggested text view right aligned to the location // entry. Only show the suggested text if we can fit the text from one // character before the end of the selection to the end of the text and the @@ -642,6 +647,7 @@ void LocationBarView::Layout() { location_bounds.height()); } } +#endif location_entry_view_->SetBounds(location_bounds); } @@ -775,13 +781,19 @@ void LocationBarView::OnAutocompleteWillAccept() { bool LocationBarView::OnCommitSuggestedText(const std::wstring& typed_text) { InstantController* instant = delegate_->GetInstant(); - if (!instant || !HasValidSuggestText()) { + if (!instant) + return false; + +#if defined(OS_WIN) + if(!HasValidSuggestText()) return false; - } location_entry_->model()->FinalizeInstantQuery( typed_text, suggested_text_view_->GetText()); return true; +#else + return location_entry_->CommitInstantSuggestion(); +#endif } void LocationBarView::OnSetSuggestedSearchText(const string16& suggested_text) { @@ -868,8 +880,12 @@ void LocationBarView::OnChanged() { } void LocationBarView::OnSelectionBoundsChanged() { +#if defined(OS_WIN) if (suggested_text_view_) suggested_text_view_->StopAnimation(); +#else + NOTREACHED(); +#endif } void LocationBarView::OnInputInProgress(bool in_progress) { @@ -1046,10 +1062,12 @@ std::string LocationBarView::GetClassName() const { bool LocationBarView::SkipDefaultKeyEventProcessing(const views::KeyEvent& e) { if (views::FocusManager::IsTabTraversalKeyEvent(e)) { +#if defined(OS_WIN) if (HasValidSuggestText()) { // Return true so that the edit sees the tab and commits the suggestion. return true; } +#endif InstantController* instant = delegate_->GetInstant(); if (instant && instant->IsCurrent()) { // Tab while showing instant commits instant immediately. @@ -1127,6 +1145,7 @@ void LocationBarView::ShowFirstRunBubble(FirstRun::BubbleType bubble_type) { } void LocationBarView::SetSuggestedText(const string16& input) { +#if defined(OS_WIN) // Don't show the suggested text if inline autocomplete is prevented. string16 text = location_entry_->model()->UseVerbatimInstant() ? string16() : input; @@ -1153,6 +1172,9 @@ void LocationBarView::SetSuggestedText(const string16& input) { Layout(); SchedulePaint(); +#else + location_entry_->SetInstantSuggestion(UTF16ToUTF8(input)); +#endif } std::wstring LocationBarView::GetInputString() const { @@ -1245,7 +1267,9 @@ void LocationBarView::OnTemplateURLModelChanged() { ShowFirstRunBubble(bubble_type_); } +#if defined(OS_WIN) bool LocationBarView::HasValidSuggestText() { return suggested_text_view_ && !suggested_text_view_->size().IsEmpty() && !suggested_text_view_->GetText().empty(); } +#endif diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.h b/chrome/browser/ui/views/location_bar/location_bar_view.h index 152e6e5..cfda874 100644 --- a/chrome/browser/ui/views/location_bar/location_bar_view.h +++ b/chrome/browser/ui/views/location_bar/location_bar_view.h @@ -42,7 +42,6 @@ class PageActionWithBadgeView; class Profile; class SelectedKeywordView; class StarView; -class SuggestedTextView; class TabContents; class TabContentsWrapper; class TemplateURLModel; @@ -52,6 +51,10 @@ class HorizontalPainter; class Label; }; +#if defined(OS_WIN) +class SuggestedTextView; +#endif + ///////////////////////////////////////////////////////////////////////////// // // LocationBarView class @@ -154,8 +157,10 @@ class LocationBarView : public LocationBar, // appears, not where the icons are shown). gfx::Point GetLocationEntryOrigin() const; +#if defined(OS_WIN) // Invoked from SuggestedTextView when the suggested text should be committed. void OnCommitSuggestedText(); +#endif // Sizing functions virtual gfx::Size GetPreferredSize(); @@ -309,14 +314,14 @@ class LocationBarView : public LocationBar, #if defined(OS_WIN) // Helper for the Mouse event handlers that does all the real work. void OnMouseEvent(const views::MouseEvent& event, UINT msg); + + // Returns true if the suggest text is valid. + bool HasValidSuggestText(); #endif // Helper to show the first run info bubble. void ShowFirstRunBubbleInternal(FirstRun::BubbleType bubble_type); - // Returns true if the suggest text is valid. - bool HasValidSuggestText(); - // Current profile. Not owned by us. Profile* profile_; @@ -370,9 +375,11 @@ class LocationBarView : public LocationBar, // Shown if the user has selected a keyword. SelectedKeywordView* selected_keyword_view_; +#if defined(OS_WIN) // View responsible for showing suggested text. This is NULL when there is no // suggested text. SuggestedTextView* suggested_text_view_; +#endif // Shown if the selected url has a corresponding keyword. KeywordHintView* keyword_hint_view_; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index a443f3a..949a322 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -4031,8 +4031,6 @@ ['include', '^browser/ui/views/location_bar/selected_keyword_view.cc'], ['include', '^browser/ui/views/location_bar/star_view.cc'], ['include', '^browser/ui/views/location_bar/star_view.h'], - ['include', '^browser/ui/views/location_bar/suggested_text_view.cc'], - ['include', '^browser/ui/views/location_bar/suggested_text_view.h'], ['include', '^browser/ui/views/location_bar_view.cc'], ['include', '^browser/ui/views/location_bar_view.h'], ['include', '^browser/ui/views/modal_dialog_delegate.cc'], |