diff options
18 files changed, 390 insertions, 272 deletions
diff --git a/chrome/browser/autocomplete/base_search_provider.cc b/chrome/browser/autocomplete/base_search_provider.cc index b39ad52..082ddba 100644 --- a/chrome/browser/autocomplete/base_search_provider.cc +++ b/chrome/browser/autocomplete/base_search_provider.cc @@ -95,6 +95,7 @@ BaseSearchProvider::SuggestResult::SuggestResult( const base::string16& suggestion, AutocompleteMatchType::Type type, const base::string16& match_contents, + const base::string16& match_contents_prefix, const base::string16& annotation, const std::string& suggest_query_params, const std::string& deletion_url, @@ -106,6 +107,7 @@ BaseSearchProvider::SuggestResult::SuggestResult( : Result(from_keyword_provider, relevance, relevance_from_server), suggestion_(suggestion), type_(type), + match_contents_prefix_(match_contents_prefix), annotation_(annotation), suggest_query_params_(suggest_query_params), deletion_url_(deletion_url), @@ -120,8 +122,20 @@ BaseSearchProvider::SuggestResult::~SuggestResult() {} void BaseSearchProvider::SuggestResult::ClassifyMatchContents( const bool allow_bolding_all, const base::string16& input_text) { - size_t input_position = match_contents_.find(input_text); - if (!allow_bolding_all && (input_position == base::string16::npos)) { + base::string16 lookup_text = input_text; + if (type_ == AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) { + const size_t contents_index = + suggestion_.length() - match_contents_.length(); + // Ensure the query starts with the input text, and ends with the match + // contents, and the input text has an overlap with contents. + if (StartsWith(suggestion_, input_text, true) && + EndsWith(suggestion_, match_contents_, true) && + (input_text.length() > contents_index)) { + lookup_text = input_text.substr(contents_index); + } + } + size_t lookup_position = match_contents_.find(lookup_text); + if (!allow_bolding_all && (lookup_position == base::string16::npos)) { // Bail if the code below to update the bolding would bold the whole // string. Note that the string may already be entirely bolded; if // so, leave it as is. @@ -132,7 +146,7 @@ void BaseSearchProvider::SuggestResult::ClassifyMatchContents( // will be highlighted, e.g. for input_text = "you" the suggestion may be // "youtube", so we'll bold the "tube" section: you*tube*. if (input_text != match_contents_) { - if (input_position == base::string16::npos) { + if (lookup_position == base::string16::npos) { // The input text is not a substring of the query string, e.g. input // text is "slasdot" and the query string is "slashdot", so we bold the // whole thing. @@ -144,13 +158,13 @@ void BaseSearchProvider::SuggestResult::ClassifyMatchContents( // short as a single character highlighted in a query suggestion result, // e.g. for input text "s" and query string "southwest airlines", it // looks odd if both the first and last s are highlighted. - if (input_position != 0) { + if (lookup_position != 0) { match_contents_class_.push_back( ACMatchClassification(0, ACMatchClassification::MATCH)); } match_contents_class_.push_back( - ACMatchClassification(input_position, ACMatchClassification::NONE)); - size_t next_fragment_position = input_position + input_text.length(); + ACMatchClassification(lookup_position, ACMatchClassification::NONE)); + size_t next_fragment_position = lookup_position + lookup_text.length(); if (next_fragment_position < match_contents_.length()) { match_contents_class_.push_back(ACMatchClassification( next_fragment_position, ACMatchClassification::MATCH)); @@ -298,6 +312,16 @@ AutocompleteMatch BaseSearchProvider::CreateSearchSuggestion( match.keyword = template_url->keyword(); match.contents = suggestion.match_contents(); match.contents_class = suggestion.match_contents_class(); + if (suggestion.type() == AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) { + match.RecordAdditionalInfo("input text", base::UTF16ToUTF8(input.text())); + match.RecordAdditionalInfo( + "match contents prefix", + base::UTF16ToUTF8(suggestion.match_contents_prefix())); + match.RecordAdditionalInfo( + "match contents start index", + static_cast<int>( + suggestion.suggestion().length() - match.contents.length())); + } if (!suggestion.annotation().empty()) match.description = suggestion.annotation(); diff --git a/chrome/browser/autocomplete/base_search_provider.h b/chrome/browser/autocomplete/base_search_provider.h index a0f1d98..2a07ed2 100644 --- a/chrome/browser/autocomplete/base_search_provider.h +++ b/chrome/browser/autocomplete/base_search_provider.h @@ -138,6 +138,7 @@ class BaseSearchProvider : public AutocompleteProvider, SuggestResult(const base::string16& suggestion, AutocompleteMatchType::Type type, const base::string16& match_contents, + const base::string16& match_contents_prefix, const base::string16& annotation, const std::string& suggest_query_params, const std::string& deletion_url, @@ -150,6 +151,9 @@ class BaseSearchProvider : public AutocompleteProvider, const base::string16& suggestion() const { return suggestion_; } AutocompleteMatchType::Type type() const { return type_; } + const base::string16& match_contents_prefix() const { + return match_contents_prefix_; + } const base::string16& annotation() const { return annotation_; } const std::string& suggest_query_params() const { return suggest_query_params_; @@ -176,6 +180,11 @@ class BaseSearchProvider : public AutocompleteProvider, AutocompleteMatchType::Type type_; + // The contents to be displayed as prefix of match contents. + // Used for postfix suggestions to display a leading ellipsis (or some + // equivalent character) to indicate omitted text. + base::string16 match_contents_prefix_; + // Optional annotation for the |match_contents_| for disambiguation. // This may be displayed in the autocomplete match contents, but is defined // separately to facilitate different formatting. diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 10e60a3..6e7497d 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -922,6 +922,7 @@ bool SearchProvider::ParseSuggestResults(base::Value* root_val, bool should_prefetch = static_cast<int>(index) == prefetch_index; base::DictionaryValue* suggestion_detail = NULL; base::string16 match_contents = suggestion; + base::string16 match_contents_prefix; base::string16 annotation; std::string suggest_query_params; std::string deletion_url; @@ -930,23 +931,21 @@ bool SearchProvider::ParseSuggestResults(base::Value* root_val, suggestion_details->GetDictionary(index, &suggestion_detail); if (suggestion_detail) { suggestion_detail->GetString("du", &deletion_url); - suggestion_detail->GetString("title", &match_contents) || - suggestion_detail->GetString("t", &match_contents); + suggestion_detail->GetString("t", &match_contents); + suggestion_detail->GetString("mp", &match_contents_prefix); // Error correction for bad data from server. if (match_contents.empty()) match_contents = suggestion; - suggestion_detail->GetString("annotation", &annotation) || - suggestion_detail->GetString("a", &annotation); - suggestion_detail->GetString("query_params", &suggest_query_params) || - suggestion_detail->GetString("q", &suggest_query_params); + suggestion_detail->GetString("a", &annotation); + suggestion_detail->GetString("q", &suggest_query_params); } } // TODO(kochi): Improve calculator suggestion presentation. results->suggest_results.push_back(SuggestResult( - suggestion, match_type, match_contents, annotation, - suggest_query_params, deletion_url, is_keyword, relevance, true, - should_prefetch, input_text)); + suggestion, match_type, match_contents, match_contents_prefix, + annotation, suggest_query_params, deletion_url, is_keyword, relevance, + true, should_prefetch, input_text)); } } @@ -996,8 +995,9 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { if (verbatim_relevance > 0) { SuggestResult verbatim( input_.text(), AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, - input_.text(), base::string16(), std::string(), std::string(), false, - verbatim_relevance, relevance_from_server, false, input_.text()); + input_.text(), base::string16(), base::string16(), std::string(), + std::string(), false, verbatim_relevance, relevance_from_server, false, + input_.text()); AddMatchToMap( verbatim, std::string(), did_not_accept_default_suggestion, &map); } @@ -1017,8 +1017,8 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { if (keyword_verbatim_relevance > 0) { SuggestResult verbatim( keyword_input_.text(), AutocompleteMatchType::SEARCH_OTHER_ENGINE, - keyword_input_.text(), base::string16(), std::string(), - std::string(), true, keyword_verbatim_relevance, + keyword_input_.text(), base::string16(), base::string16(), + std::string(), std::string(), true, keyword_verbatim_relevance, keyword_relevance_from_server, false, keyword_input_.text()); AddMatchToMap( verbatim, std::string(), did_not_accept_keyword_suggestion, &map); @@ -1370,8 +1370,8 @@ SearchProvider::SuggestResults SearchProvider::ScoreHistoryResults( prevent_search_history_inlining); scored_results.push_back(SuggestResult( i->term, AutocompleteMatchType::SEARCH_HISTORY, i->term, - base::string16(), std::string(), std::string(), is_keyword, relevance, - false, false, input_text)); + base::string16(), base::string16(), std::string(), std::string(), + is_keyword, relevance, false, false, input_text)); } // History returns results sorted for us. However, we may have docked some diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index 2cb68a4..9ba947e 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -3411,9 +3411,10 @@ TEST_F(SearchProviderTest, RemoveStaleResultsTest) { provider_->default_results_.suggest_results.push_back( SearchProvider::SuggestResult( ASCIIToUTF16(suggestion), AutocompleteMatchType::SEARCH_SUGGEST, - ASCIIToUTF16(suggestion), base::string16(), std::string(), - std::string(), false, cases[i].results[j].relevance, false, - false, ASCIIToUTF16(cases[i].omnibox_input))); + ASCIIToUTF16(suggestion), base::string16(), base::string16(), + std::string(), std::string(), false, + cases[i].results[j].relevance, false, false, + ASCIIToUTF16(cases[i].omnibox_input))); } } diff --git a/chrome/browser/autocomplete/zero_suggest_provider.cc b/chrome/browser/autocomplete/zero_suggest_provider.cc index f488c9e..32b99b4 100644 --- a/chrome/browser/autocomplete/zero_suggest_provider.cc +++ b/chrome/browser/autocomplete/zero_suggest_provider.cc @@ -207,7 +207,7 @@ void ZeroSuggestProvider::AddSuggestResultsToMap( // a different query -- create correct objects to begin with. const SuggestResult suggestion( query_string, AutocompleteMatchType::SEARCH_SUGGEST, query_string, - base::string16(), std::string(), std::string(), false, + base::string16(), base::string16(), std::string(), std::string(), false, results[i].relevance(), true, false, query_string); AddMatchToMap(suggestion, std::string(), i, map); } @@ -345,8 +345,8 @@ void ZeroSuggestProvider::ParseSuggestResults(const base::Value& root_val) { } else { results_.suggest_results.push_back(SuggestResult( result, AutocompleteMatchType::SEARCH_SUGGEST, result, - base::string16(), std::string(), std::string(), false, relevance, - relevances != NULL, false, current_query_string16)); + base::string16(), base::string16(), std::string(), std::string(), + false, relevance, relevances != NULL, false, current_query_string16)); } } } diff --git a/chrome/browser/history/shortcuts_backend.cc b/chrome/browser/history/shortcuts_backend.cc index a5abb3fe..2d95858 100644 --- a/chrome/browser/history/shortcuts_backend.cc +++ b/chrome/browser/history/shortcuts_backend.cc @@ -55,6 +55,10 @@ AutocompleteMatch::Type GetTypeForShortcut(AutocompleteMatch::Type type) { case AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED: case AutocompleteMatchType::SEARCH_SUGGEST: + case AutocompleteMatchType::SEARCH_SUGGEST_ENTITY: + case AutocompleteMatchType::SEARCH_SUGGEST_INFINITE: + case AutocompleteMatchType::SEARCH_SUGGEST_PERSONALIZED: + case AutocompleteMatchType::SEARCH_SUGGEST_PROFILE: return AutocompleteMatchType::SEARCH_HISTORY; default: diff --git a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc index a6f839b..1e93f29 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc @@ -4,6 +4,8 @@ #include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h" +#include <algorithm> + #include "chrome/browser/search/search.h" #include "chrome/browser/themes/theme_properties.h" #include "chrome/browser/ui/omnibox/omnibox_view.h" @@ -85,7 +87,7 @@ void OmniboxPopupContentsView::Init() { // necessarily our final class yet, and we may have subclasses // overriding CreateResultView. for (size_t i = 0; i < AutocompleteResult::kMaxMatches; ++i) { - OmniboxResultView* result_view = CreateResultView(this, i, font_list_); + OmniboxResultView* result_view = CreateResultView(i, font_list_); result_view->SetVisible(false); AddChildViewAt(result_view, static_cast<int>(i)); } @@ -170,11 +172,18 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() { // Update the match cached by each row, in the process of doing so make sure // we have enough row views. const size_t result_size = model_->result().size(); + max_match_contents_width_ = 0; for (size_t i = 0; i < result_size; ++i) { OmniboxResultView* view = result_view_at(i); - view->SetMatch(GetMatchAtIndex(i)); + const AutocompleteMatch& match = GetMatchAtIndex(i); + view->SetMatch(match); view->SetVisible(i >= hidden_matches); + if (match.type == AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) { + max_match_contents_width_ = std::max( + max_match_contents_width_, view->GetMatchContentsWidth()); + } } + for (size_t i = result_size; i < AutocompleteResult::kMaxMatches; ++i) child_at(i)->SetVisible(false); @@ -402,10 +411,9 @@ int OmniboxPopupContentsView::CalculatePopupHeight() { } OmniboxResultView* OmniboxPopupContentsView::CreateResultView( - OmniboxResultViewModel* model, int model_index, const gfx::FontList& font_list) { - return new OmniboxResultView(model, model_index, location_bar_view_, + return new OmniboxResultView(this, model_index, location_bar_view_, font_list); } diff --git a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h index 17793a5..e54b5b7 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h +++ b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h @@ -8,7 +8,6 @@ #include "base/memory/weak_ptr.h" #include "chrome/browser/ui/omnibox/omnibox_popup_model.h" #include "chrome/browser/ui/omnibox/omnibox_popup_view.h" -#include "chrome/browser/ui/views/omnibox/omnibox_result_view_model.h" #include "ui/base/window_open_disposition.h" #include "ui/gfx/animation/animation_delegate.h" #include "ui/gfx/animation/slide_animation.h" @@ -24,7 +23,6 @@ class Profile; // A view representing the contents of the autocomplete popup. class OmniboxPopupContentsView : public views::View, - public OmniboxResultViewModel, public OmniboxPopupView, public gfx::AnimationDelegate { public: @@ -48,11 +46,6 @@ class OmniboxPopupContentsView : public views::View, virtual void PaintUpdatesNow() OVERRIDE; virtual void OnDragCanceled() OVERRIDE; - // Overridden from OmniboxResultViewModel: - virtual bool IsSelectedIndex(size_t index) const OVERRIDE; - virtual bool IsHoveredIndex(size_t index) const OVERRIDE; - virtual gfx::Image GetIconIfExtensionMatch(size_t index) const OVERRIDE; - // Overridden from gfx::AnimationDelegate: virtual void AnimationProgressed(const gfx::Animation* animation) OVERRIDE; @@ -73,6 +66,14 @@ class OmniboxPopupContentsView : public views::View, // Overridden from ui::EventHandler: virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE; + bool IsSelectedIndex(size_t index) const; + bool IsHoveredIndex(size_t index) const; + gfx::Image GetIconIfExtensionMatch(size_t index) const; + + int max_match_contents_width() const { + return max_match_contents_width_; + } + protected: OmniboxPopupContentsView(const gfx::FontList& font_list, OmniboxView* omnibox_view, @@ -86,8 +87,7 @@ class OmniboxPopupContentsView : public views::View, // Calculates the height needed to show all the results in the model. virtual int CalculatePopupHeight(); - virtual OmniboxResultView* CreateResultView(OmniboxResultViewModel* model, - int model_index, + virtual OmniboxResultView* CreateResultView(int model_index, const gfx::FontList& font_list); // Overridden from views::View: @@ -169,6 +169,11 @@ class OmniboxPopupContentsView : public views::View, // Amount of extra padding to add to the popup on the top and bottom. int outside_vertical_padding_; + // When the dropdown is not wide enough while displaying postfix suggestions, + // we use the width of widest match contents to shift the suggestions so that + // the widest suggestion just reaches the end edge. + int max_match_contents_width_; + DISALLOW_COPY_AND_ASSIGN(OmniboxPopupContentsView); }; diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc index 9c830dd..690c1b2 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc +++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc @@ -15,9 +15,11 @@ #include "base/i18n/bidi_line_iterator.h" #include "base/memory/scoped_vector.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/string_util.h" #include "chrome/browser/ui/omnibox/omnibox_popup_model.h" #include "chrome/browser/ui/views/location_bar/location_bar_view.h" -#include "chrome/browser/ui/views/omnibox/omnibox_result_view_model.h" +#include "chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" #include "ui/base/l10n/l10n_util.h" @@ -25,6 +27,7 @@ #include "ui/gfx/canvas.h" #include "ui/gfx/color_utils.h" #include "ui/gfx/image/image.h" +#include "ui/gfx/range/range.h" #include "ui/gfx/render_text.h" #include "ui/gfx/text_elider.h" #include "ui/gfx/text_utils.h" @@ -40,8 +43,6 @@ namespace { -const base::char16 kEllipsis[] = { 0x2026, 0x0 }; - // The minimum distance between the top and bottom of the {icon|text} and the // top or bottom of the row. const int kMinimumIconVerticalPadding = 2; @@ -52,22 +53,6 @@ const int kMinimumTextVerticalPadding = 3; //////////////////////////////////////////////////////////////////////////////// // OmniboxResultView, public: -// Precalculated data used to draw a complete visual run within the match. -// This will include all or part of at least one, and possibly several, -// classifications. -struct OmniboxResultView::RunData { - RunData() : run_start(0), visual_order(0), is_rtl(false), pixel_width(0) {} - - size_t run_start; // Offset within the match text where this run begins. - int visual_order; // Where this run occurs in visual order. The earliest - // run drawn is run 0. - bool is_rtl; - int pixel_width; - - // Styled text classification pieces within this run, in logical order. - Classifications classifications; -}; - // This class is a utility class for calculations affected by whether the result // view is horizontally mirrored. The drawing functions can be written as if // all drawing occurs left-to-right, and then use this class to get the actual @@ -104,7 +89,7 @@ class OmniboxResultView::MirroringContext { DISALLOW_COPY_AND_ASSIGN(MirroringContext); }; -OmniboxResultView::OmniboxResultView(OmniboxResultViewModel* model, +OmniboxResultView::OmniboxResultView(OmniboxPopupContentsView* model, int model_index, LocationBarView* location_bar_view, const gfx::FontList& font_list) @@ -118,8 +103,6 @@ OmniboxResultView::OmniboxResultView(OmniboxResultViewModel* model, font_height_( std::max(font_list.GetHeight(), font_list.DeriveWithStyle(gfx::Font::BOLD).GetHeight())), - ellipsis_width_(gfx::GetStringWidth(base::string16(kEllipsis), - font_list)), mirroring_context_(new MirroringContext()), keyword_icon_(new views::ImageView()), animation_(new gfx::SlideAnimation(this)) { @@ -130,6 +113,8 @@ OmniboxResultView::OmniboxResultView(OmniboxResultViewModel* model, AutocompleteMatch::TypeToIcon( AutocompleteMatchType::URL_WHAT_YOU_TYPED))->width(); } + if (ellipsis_width_ == 0) + ellipsis_width_ = CreateRenderText(gfx::kEllipsisUTF16)->GetContentWidth(); keyword_icon_->set_owned_by_client(); keyword_icon_->EnableCanvasFlippingForRTLUI(true); keyword_icon_->SetImage(GetKeywordIcon()); @@ -182,9 +167,11 @@ SkColor OmniboxResultView::GetColor( void OmniboxResultView::SetMatch(const AutocompleteMatch& match) { match_ = match; + match_contents_render_text_.reset(); animation_->Reset(); - if (match.associated_keyword.get()) { + AutocompleteMatch* associated_keyword_match = match_.associated_keyword.get(); + if (associated_keyword_match) { keyword_icon_->SetImage(GetKeywordIcon()); if (!keyword_icon_->parent()) @@ -193,6 +180,9 @@ void OmniboxResultView::SetMatch(const AutocompleteMatch& match) { RemoveChildView(keyword_icon_.get()); } + render_associated_keyword_match_ = + associated_keyword_match && keyword_icon_->x() <= icon_bounds_.right(); + RenderMatchContents(); Layout(); } @@ -227,30 +217,203 @@ int OmniboxResultView::GetTextHeight() const { return font_height_; } -void OmniboxResultView::PaintMatch(gfx::Canvas* canvas, - const AutocompleteMatch& match, - int x) { - x = DrawString(canvas, match.contents, match.contents_class, false, x, - text_bounds_.y()); - - // Paint the description. - // TODO(pkasting): Because we paint in multiple separate pieces, we can wind - // up with no space even for an ellipsis for one or both of these pieces. - // Instead, we should paint the entire match as a single long string. This - // would also let us use a more properly-localizable string than we get with - // just the IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR. - if (!match.description.empty()) { - base::string16 separator = - l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR); - ACMatchClassifications classifications; - classifications.push_back( - ACMatchClassification(0, ACMatchClassification::NONE)); - x = DrawString(canvas, separator, classifications, true, x, - text_bounds_.y()); - - DrawString(canvas, match.description, match.description_class, true, x, - text_bounds_.y()); +void OmniboxResultView::PaintMatch(gfx::Canvas* canvas, int x) { + const AutocompleteMatch& match = display_match(); + int y = text_bounds_.y(); + x = DrawRenderText(canvas, RenderMatchContents(), true, x, y); + if (match.description.empty()) + return; + + const base::string16& separator = + l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR); + scoped_ptr<gfx::RenderText> separator_render_text( + CreateRenderText(separator)); + separator_render_text->SetColor(GetColor(GetState(), DIMMED_TEXT)); + + scoped_ptr<gfx::RenderText> description_render_text( + CreateRenderText(match.description)); + ApplyClassifications(description_render_text.get(), match.description_class, + true); + + const int min_desc_width = separator_render_text->GetContentWidth() + + std::min(description_render_text->GetContentWidth(), ellipsis_width_); + if (mirroring_context_->remaining_width(x) < min_desc_width) + return; + + x = DrawRenderText(canvas, separator_render_text.get(), false, x, y); + + DrawRenderText(canvas, description_render_text.get(), false, x, y); +} + +int OmniboxResultView::DrawRenderText( + gfx::Canvas* canvas, + gfx::RenderText* render_text, + bool contents, + int x, + int y) const { + DCHECK(!render_text->text().empty()); + + const int remaining_width = mirroring_context_->remaining_width(x); + const int content_width = render_text->GetContentWidth(); + int right_x = x + std::min(remaining_width, content_width); + + const AutocompleteMatch& match = display_match(); + + // Infinite suggestions should appear with the leading ellipses vertically + // stacked. + if (contents && + (match.type == AutocompleteMatchType::SEARCH_SUGGEST_INFINITE)) { + // When the directionality of suggestion doesn't match the UI, we try to + // vertically stack the ellipsis by restricting the end edge (right_x). + const bool is_ui_rtl = base::i18n::IsRTL(); + const bool is_match_contents_rtl = + (render_text->GetTextDirection() == base::i18n::RIGHT_TO_LEFT); + const int offset = GetDisplayOffset(is_ui_rtl, is_match_contents_rtl); + + scoped_ptr<gfx::RenderText> prefix_render_text( + CreateRenderText(base::UTF8ToUTF16( + match.GetAdditionalInfo("match contents prefix")))); + const int prefix_width = prefix_render_text->GetContentWidth(); + int prefix_x = x; + + const int max_match_contents_width = model_->max_match_contents_width(); + + if (is_ui_rtl != is_match_contents_rtl) { + // RTL infinite suggestions appear near the left edge in LTR UI, while LTR + // infinite suggestions appear near the right edge in RTL UI. This is + // against the natural horizontal alignment of the text. We reduce the + // width of the box for suggestion display, so that the suggestions appear + // in correct confines. This reduced width allows us to modify the text + // alignment (see below). + right_x = x + std::min(remaining_width - prefix_width, + std::max(offset, max_match_contents_width)); + prefix_x = right_x; + // We explicitly set the horizontal alignment so that when LTR suggestions + // show in RTL UI (or vice versa), their ellipses appear stacked in a + // single column. + render_text->SetHorizontalAlignment( + is_match_contents_rtl ? gfx::ALIGN_RIGHT : gfx::ALIGN_LEFT); + } else { + // If the dropdown is wide enough, place the ellipsis at the position + // where the omitted text would have ended. Otherwise reduce the offset of + // the ellipsis such that the widest suggestion reaches the end of the + // dropdown. + const int start_offset = std::max(prefix_width, + std::min(remaining_width - (prefix_width + max_match_contents_width), + offset)); + right_x = x + std::min(remaining_width, start_offset + content_width); + x += start_offset; + prefix_x = x - prefix_width; + } + prefix_render_text->SetDirectionalityMode(is_match_contents_rtl ? + gfx::DIRECTIONALITY_FORCE_RTL : gfx::DIRECTIONALITY_FORCE_LTR); + prefix_render_text->SetHorizontalAlignment( + is_match_contents_rtl ? gfx::ALIGN_RIGHT : gfx::ALIGN_LEFT); + prefix_render_text->SetDisplayRect(gfx::Rect( + mirroring_context_->mirrored_left_coord( + prefix_x, prefix_x + prefix_width), y, + prefix_width, height())); + prefix_render_text->Draw(canvas); + } + + // Set the display rect to trigger eliding. + render_text->SetDisplayRect(gfx::Rect( + mirroring_context_->mirrored_left_coord(x, right_x), y, + right_x - x, height())); + render_text->Draw(canvas); + return right_x; +} + +scoped_ptr<gfx::RenderText> OmniboxResultView::CreateRenderText( + const base::string16& text) const { + scoped_ptr<gfx::RenderText> render_text(gfx::RenderText::CreateInstance()); + render_text->SetFontList(font_list_); + render_text->SetText(text); + render_text->SetElideBehavior(gfx::ELIDE_AT_END); + return render_text.Pass(); +} + +void OmniboxResultView::ApplyClassifications( + gfx::RenderText* render_text, + const ACMatchClassifications& classifications, + bool force_dim) const { + const size_t text_length = render_text->text().length(); + for (size_t i = 0; i < classifications.size(); ++i) { + const size_t text_start = classifications[i].offset; + if (text_start >= text_length) + break; + + const size_t text_end = (i < (classifications.size() - 1)) ? + std::min(classifications[i + 1].offset, text_length) : + text_length; + const gfx::Range current_range(text_start, text_end); + + // Calculate style-related data. + if (classifications[i].style & ACMatchClassification::MATCH) + render_text->ApplyStyle(gfx::BOLD, true, current_range); + + ColorKind color_kind = TEXT; + if (classifications[i].style & ACMatchClassification::URL) { + color_kind = URL; + // Consider logical string for domain "ABC.com×™/hello" where ABC are + // Hebrew (RTL) characters. This string should ideally show as + // "CBA.com/hello". If we do not force LTR on URL, it will appear as + // "com/hello.CBA". + // With IDN and RTL TLDs, it might be okay to allow RTL rendering of URLs, + // but it still has some pitfalls like : + // ABC.COM/abc-pqr/xyz/FGH will appear as HGF/abc-pqr/xyz/MOC.CBA which + // really confuses the path hierarchy of the URL. + // Also, if the URL supports https, the appearance will change into LTR + // directionality. + // In conclusion, LTR rendering of URL is probably the safest bet. + render_text->SetDirectionalityMode(gfx::DIRECTIONALITY_FORCE_LTR); + } else if (force_dim || + (classifications[i].style & ACMatchClassification::DIM)) { + color_kind = DIMMED_TEXT; + } + render_text->ApplyColor(GetColor(GetState(), color_kind), current_range); + } +} + +gfx::RenderText* OmniboxResultView::RenderMatchContents() { + if (!match_contents_render_text_) { + const AutocompleteMatch& match = display_match(); + match_contents_render_text_.reset( + CreateRenderText(match.contents).release()); + ApplyClassifications(match_contents_render_text_.get(), + match.contents_class, false); } + return match_contents_render_text_.get(); +} + +int OmniboxResultView::GetMatchContentsWidth() const { + return match_contents_render_text_->GetContentWidth(); +} + +// TODO(skanuj): This is probably identical across all OmniboxResultView rows in +// the omnibox dropdown. Consider sharing the result. +int OmniboxResultView::GetDisplayOffset( + bool is_ui_rtl, + bool is_match_contents_rtl) const { + const AutocompleteMatch& match = display_match(); + if (match.type != AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) + return 0; + + const base::string16& input_text = + base::UTF8ToUTF16(match.GetAdditionalInfo("input text")); + int contents_start_index = 0; + base::StringToInt(match.GetAdditionalInfo("match contents start index"), + &contents_start_index); + + scoped_ptr<gfx::RenderText> input_render_text(CreateRenderText(input_text)); + const gfx::Range& glyph_bounds = + input_render_text->GetGlyphBounds(contents_start_index); + const int start_padding = is_match_contents_rtl ? + std::max(glyph_bounds.start(), glyph_bounds.end()) : + std::min(glyph_bounds.start(), glyph_bounds.end()); + + return is_ui_rtl ? + (input_render_text->GetContentWidth() - start_padding) : start_padding; } // static @@ -287,19 +450,10 @@ void OmniboxResultView::CommonInitColors(const ui::NativeTheme* theme, } // static -bool OmniboxResultView::SortRunsLogically(const RunData& lhs, - const RunData& rhs) { - return lhs.run_start < rhs.run_start; -} - -// static -bool OmniboxResultView::SortRunsVisually(const RunData& lhs, - const RunData& rhs) { - return lhs.visual_order < rhs.visual_order; -} +int OmniboxResultView::default_icon_size_ = 0; // static -int OmniboxResultView::default_icon_size_ = 0; +int OmniboxResultView::ellipsis_width_ = 0; gfx::ImageSkia OmniboxResultView::GetIcon() const { const gfx::Image image = model_->GetIconIfExtensionMatch(model_index_); @@ -337,81 +491,6 @@ const gfx::ImageSkia* OmniboxResultView::GetKeywordIcon() const { (GetState() == SELECTED) ? IDR_OMNIBOX_TTS_SELECTED : IDR_OMNIBOX_TTS); } -int OmniboxResultView::DrawString( - gfx::Canvas* canvas, - const base::string16& text, - const ACMatchClassifications& classifications, - bool force_dim, - int x, - int y) { - if (text.empty()) - return x; - - // Check whether or not this text is a URL. URLs are always displayed LTR - // regardless of locale. - bool is_url = true; - for (ACMatchClassifications::const_iterator i(classifications.begin()); - i != classifications.end(); ++i) { - if (!(i->style & ACMatchClassification::URL)) { - is_url = false; - break; - } - } - - scoped_ptr<gfx::RenderText> render_text(gfx::RenderText::CreateInstance()); - const size_t text_length = text.length(); - render_text->SetText(text); - render_text->SetFontList(font_list_); - render_text->SetMultiline(false); - render_text->SetCursorEnabled(false); - if (is_url) - render_text->SetDirectionalityMode(gfx::DIRECTIONALITY_FORCE_LTR); - - // Apply classifications. - for (size_t i = 0; i < classifications.size(); ++i) { - const size_t text_start = classifications[i].offset; - if (text_start >= text_length) - break; - - const size_t text_end = (i < (classifications.size() - 1)) ? - std::min(classifications[i + 1].offset, text_length) : text_length; - const gfx::Range current_range(text_start, text_end); - - // Calculate style-related data. - if (classifications[i].style & ACMatchClassification::MATCH) - render_text->ApplyStyle(gfx::BOLD, true, current_range); - - ColorKind color_kind = TEXT; - if (classifications[i].style & ACMatchClassification::URL) { - color_kind = URL; - } else if (force_dim || - (classifications[i].style & ACMatchClassification::DIM)) { - color_kind = DIMMED_TEXT; - } - render_text->ApplyColor(GetColor(GetState(), color_kind), current_range); - } - - int remaining_width = mirroring_context_->remaining_width(x); - - // No need to try anything if we can't even show a solitary character. - if ((text_length == 1) && - (remaining_width < render_text->GetContentWidth())) { - return x; - } - - if (render_text->GetContentWidth() > remaining_width) - render_text->SetElideBehavior(gfx::ELIDE_AT_END); - - // Set the display rect to trigger eliding. - render_text->SetDisplayRect(gfx::Rect( - mirroring_context_->mirrored_left_coord(x, x + remaining_width), y, - remaining_width, height())); - render_text->set_clip_to_display_rect(true); - render_text->Draw(canvas); - - // Need to call GetContentWidth again as the SetDisplayRect may modify it. - return x + render_text->GetContentWidth(); -} void OmniboxResultView::Layout() { const gfx::ImageSkia icon = GetIcon(); @@ -452,24 +531,16 @@ void OmniboxResultView::OnPaint(gfx::Canvas* canvas) { if (state != NORMAL) canvas->DrawColor(GetColor(state, BACKGROUND)); - if (!match_.associated_keyword.get() || - keyword_icon_->x() > icon_bounds_.right()) { + if (!render_associated_keyword_match_) { // Paint the icon. canvas->DrawImageInt(GetIcon(), GetMirroredXForRect(icon_bounds_), icon_bounds_.y()); - - // Paint the text. - int x = GetMirroredXForRect(text_bounds_); - mirroring_context_->Initialize(x, text_bounds_.width()); - PaintMatch(canvas, match_, x); - } - - if (match_.associated_keyword.get()) { - // Paint the keyword text. - int x = GetMirroredXForRect(keyword_text_bounds_); - mirroring_context_->Initialize(x, keyword_text_bounds_.width()); - PaintMatch(canvas, *match_.associated_keyword.get(), x); } + const gfx::Rect& text_bounds = render_associated_keyword_match_ ? + keyword_text_bounds_ : text_bounds_; + int x = GetMirroredXForRect(text_bounds); + mirroring_context_->Initialize(x, text_bounds.width()); + PaintMatch(canvas, x); } void OmniboxResultView::AnimationProgressed(const gfx::Animation* animation) { diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.h b/chrome/browser/ui/views/omnibox/omnibox_result_view.h index 886e1d5..4c02911 100644 --- a/chrome/browser/ui/views/omnibox/omnibox_result_view.h +++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.h @@ -17,7 +17,7 @@ #include "ui/views/view.h" class LocationBarView; -class OmniboxResultViewModel; +class OmniboxPopupContentsView; namespace gfx { class Canvas; @@ -45,7 +45,7 @@ class OmniboxResultView : public views::View, NUM_KINDS }; - OmniboxResultView(OmniboxResultViewModel* model, + OmniboxResultView(OmniboxPopupContentsView* model, int model_index, LocationBarView* location_bar_view, const gfx::FontList& font_list); @@ -71,21 +71,34 @@ class OmniboxResultView : public views::View, // class, this is the height of one line of text. virtual int GetTextHeight() const; + // Returns the display width required for the match contents. + int GetMatchContentsWidth() const; + protected: - virtual void PaintMatch(gfx::Canvas* canvas, - const AutocompleteMatch& match, - int x); - - // Draws the specified |text| into the canvas, using highlighting provided by - // |classifications|. If |force_dim| is true, ACMatchClassification::DIM is - // added to all of the classifications. Returns the x position to the right - // of the string. - int DrawString(gfx::Canvas* canvas, - const base::string16& text, - const ACMatchClassifications& classifications, - bool force_dim, - int x, - int y); + virtual void PaintMatch(gfx::Canvas* canvas, int x); + + // Draws given |render_text| on |canvas| at given location (|x|, |y|). + // |contents| determines any formatting difference between contents and + // description parts of the omnibox result (see AutocompleteMatch). + int DrawRenderText(gfx::Canvas* canvas, + gfx::RenderText* render_text, + bool contents, + int x, + int y) const; + + // Creates a RenderText with given |text| and rendering defaults. + scoped_ptr<gfx::RenderText> CreateRenderText( + const base::string16& text) const; + + // Applies styles specified by |classifications| and |force_dim| in the range + // from |range_start| to |range_end| in the |render_text|. + void ApplyClassifications( + gfx::RenderText* render_text, + const ACMatchClassifications& classifications, + bool force_dim) const; + + // Renders match contents at a suitable location in the bounds of this view. + gfx::RenderText* RenderMatchContents(); const gfx::Rect& text_bounds() const { return text_bounds_; } @@ -95,19 +108,17 @@ class OmniboxResultView : public views::View, minimum_text_vertical_padding_ = value; } - private: - struct RunData; - typedef std::vector<RunData> Runs; - typedef std::vector<gfx::RenderText*> Classifications; + // Returns the match to be rendered for this row. + const AutocompleteMatch& display_match() const { + return render_associated_keyword_match_ ? + *match_.associated_keyword.get() : match_; + } + private: // Common initialization code of the colors returned by GetColors(). static void CommonInitColors(const ui::NativeTheme* theme, SkColor colors[][NUM_KINDS]); - // Predicate functions for use when sorting the runs. - static bool SortRunsLogically(const RunData& lhs, const RunData& rhs); - static bool SortRunsVisually(const RunData& lhs, const RunData& rhs); - gfx::ImageSkia GetIcon() const; const gfx::ImageSkia* GetKeywordIcon() const; @@ -119,15 +130,22 @@ class OmniboxResultView : public views::View, // gfx::AnimationDelegate: virtual void AnimationProgressed(const gfx::Animation* animation) OVERRIDE; + // Returns the offset at which the suggestion should be displayed within the + // text bounds. The directionality of UI and match contents is used to + // determine the offset relative to the correct edge. + int GetDisplayOffset(bool is_ui_rtl, bool is_match_contents_rtl) const; + static int default_icon_size_; + static int ellipsis_width_; + // Default values cached here, may be overridden using the setters above. int edge_item_padding_; int item_padding_; int minimum_text_vertical_padding_; // This row's model and model index. - OmniboxResultViewModel* model_; + OmniboxPopupContentsView* model_; size_t model_index_; LocationBarView* location_bar_view_; @@ -135,15 +153,16 @@ class OmniboxResultView : public views::View, const gfx::FontList font_list_; int font_height_; - // Width of the ellipsis in the normal font. - int ellipsis_width_; - // A context used for mirroring regions. class MirroringContext; scoped_ptr<MirroringContext> mirroring_context_; AutocompleteMatch match_; + // Whether the associated keyword match should be rendered instead of the + // original match. + bool render_associated_keyword_match_; + gfx::Rect text_bounds_; gfx::Rect icon_bounds_; @@ -152,6 +171,8 @@ class OmniboxResultView : public views::View, scoped_ptr<gfx::SlideAnimation> animation_; + scoped_ptr<gfx::RenderText> match_contents_render_text_; + DISALLOW_COPY_AND_ASSIGN(OmniboxResultView); }; diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view_model.h b/chrome/browser/ui/views/omnibox/omnibox_result_view_model.h deleted file mode 100644 index 0a1db3d..0000000 --- a/chrome/browser/ui/views/omnibox/omnibox_result_view_model.h +++ /dev/null @@ -1,27 +0,0 @@ -// 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. - -#ifndef CHROME_BROWSER_UI_VIEWS_OMNIBOX_OMNIBOX_RESULT_VIEW_MODEL_H_ -#define CHROME_BROWSER_UI_VIEWS_OMNIBOX_OMNIBOX_RESULT_VIEW_MODEL_H_ - -namespace gfx { -class Image; -} - -// An interface implemented by an object that provides data to populate -// individual result views. -class OmniboxResultViewModel { - public: - // Returns true if the index is selected. - virtual bool IsSelectedIndex(size_t index) const = 0; - - // Returns true if the index is hovered. - virtual bool IsHoveredIndex(size_t index) const = 0; - - // If |index| is a match from an extension, returns the extension icon; - // otherwise returns an empty gfx::Image. - virtual gfx::Image GetIconIfExtensionMatch(size_t index) const = 0; -}; - -#endif // CHROME_BROWSER_UI_VIEWS_OMNIBOX_OMNIBOX_RESULT_VIEW_MODEL_H_ diff --git a/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.cc b/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.cc index 1a94ea3..8394a1b 100644 --- a/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.cc +++ b/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.cc @@ -11,13 +11,14 @@ #include "ui/gfx/font_list.h" #include "ui/gfx/path.h" #include "ui/gfx/rect.h" +#include "ui/gfx/render_text.h" #include "ui/gfx/size.h" #include "ui/views/view.h" // TouchOmniboxResultView ------------------------------------------------ TouchOmniboxResultView::TouchOmniboxResultView( - OmniboxResultViewModel* model, + OmniboxPopupContentsView* model, int model_index, LocationBarView* location_bar_view, const gfx::FontList& font_list) @@ -30,15 +31,17 @@ TouchOmniboxResultView::TouchOmniboxResultView( TouchOmniboxResultView::~TouchOmniboxResultView() { } -void TouchOmniboxResultView::PaintMatch(gfx::Canvas* canvas, - const AutocompleteMatch& match, - int x) { +void TouchOmniboxResultView::PaintMatch(gfx::Canvas* canvas, int x) { + const AutocompleteMatch& match = display_match(); int y = text_bounds().y(); if (!match.description.empty()) { // We use our base class's GetTextHeight below because we need the height // of a single line of text. - DrawString(canvas, match.description, match.description_class, true, x, y); + scoped_ptr<gfx::RenderText> render_text( + CreateRenderText(match.description)); + ApplyClassifications(render_text.get(), match.description_class, true); + DrawRenderText(canvas, render_text.get(), false, x, y); y += OmniboxResultView::GetTextHeight(); } else { // When we have only one line of content (no description), we center the @@ -46,7 +49,7 @@ void TouchOmniboxResultView::PaintMatch(gfx::Canvas* canvas, y += OmniboxResultView::GetTextHeight() / 2; } - DrawString(canvas, match.contents, match.contents_class, false, x, y); + DrawRenderText(canvas, RenderMatchContents(), true, x, y); } int TouchOmniboxResultView::GetTextHeight() const { @@ -100,10 +103,9 @@ void TouchOmniboxPopupContentsView::PaintResultViews(gfx::Canvas* canvas) { } OmniboxResultView* TouchOmniboxPopupContentsView::CreateResultView( - OmniboxResultViewModel* model, int model_index, const gfx::FontList& font_list) { - return new TouchOmniboxResultView(model, model_index, location_bar_view(), + return new TouchOmniboxResultView(this, model_index, location_bar_view(), font_list); } diff --git a/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.h b/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.h index e87745c..2a84834 100644 --- a/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.h +++ b/chrome/browser/ui/views/omnibox/touch_omnibox_popup_contents_view.h @@ -22,7 +22,7 @@ class View; class TouchOmniboxResultView : public OmniboxResultView { public: - TouchOmniboxResultView(OmniboxResultViewModel* model, + TouchOmniboxResultView(OmniboxPopupContentsView* model, int model_index, LocationBarView* location_bar_view, const gfx::FontList& font_list); @@ -31,9 +31,7 @@ class TouchOmniboxResultView : public OmniboxResultView { virtual ~TouchOmniboxResultView(); // OmniboxResultView: - virtual void PaintMatch(gfx::Canvas* canvas, - const AutocompleteMatch& match, - int x) OVERRIDE; + virtual void PaintMatch(gfx::Canvas* canvas, int x) OVERRIDE; virtual int GetTextHeight() const OVERRIDE; DISALLOW_COPY_AND_ASSIGN(TouchOmniboxResultView); @@ -55,7 +53,6 @@ class TouchOmniboxPopupContentsView // OmniboxPopupContentsView: virtual void PaintResultViews(gfx::Canvas* canvas) OVERRIDE; virtual OmniboxResultView* CreateResultView( - OmniboxResultViewModel* model, int model_index, const gfx::FontList& font_list) OVERRIDE; diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index c6d8fa3..5e30700 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -2046,7 +2046,6 @@ 'browser/ui/views/omnibox/omnibox_popup_contents_view.h', 'browser/ui/views/omnibox/omnibox_result_view.cc', 'browser/ui/views/omnibox/omnibox_result_view.h', - 'browser/ui/views/omnibox/omnibox_result_view_model.h', 'browser/ui/views/omnibox/omnibox_view_views.cc', 'browser/ui/views/omnibox/omnibox_view_views.h', 'browser/ui/views/omnibox/touch_omnibox_popup_contents_view.cc', diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc index 2a65660..0e5bfd2 100644 --- a/ui/gfx/render_text.cc +++ b/ui/gfx/render_text.cc @@ -878,7 +878,7 @@ void RenderText::SetSelectionModel(const SelectionModel& model) { } const base::string16& RenderText::GetLayoutText() const { - return layout_text_.empty() ? text_ : layout_text_; + return layout_text_; } const BreakList<size_t>& RenderText::GetLineBreaks() { @@ -1112,9 +1112,11 @@ void RenderText::UpdateLayoutText() { if (layout_text_.length() > cp_start) layout_text_.replace(cp_start, 1, text_.substr(start, end - start)); } + } else { + layout_text_ = text_; } - const base::string16& text = GetLayoutText(); + const base::string16& text = layout_text_; if (truncate_length_ > 0 && truncate_length_ < text.length()) { // Truncate the text at a valid character break and append an ellipsis. icu::StringCharacterIterator iter(text.c_str()); @@ -1123,13 +1125,12 @@ void RenderText::UpdateLayoutText() { } if (elide_behavior_ != NO_ELIDE && display_rect_.width() > 0 && - !GetLayoutText().empty() && GetContentWidth() > display_rect_.width()) { - base::string16 elided_text = ElideText(GetLayoutText()); - + !layout_text_.empty() && GetContentWidth() > display_rect_.width()) { // This doesn't trim styles so ellipsis may get rendered as a different // style than the preceding text. See crbug.com/327850. - layout_text_.assign(elided_text); + layout_text_.assign(ElideText(layout_text_)); } + ResetLayout(); } @@ -1177,6 +1178,7 @@ base::string16 RenderText::ElideText(const base::string16& text) { // Use binary search to compute the elided text. size_t lo = 0; size_t hi = text.length() - 1; + const base::i18n::TextDirection text_direction = GetTextDirection(); for (size_t guess = (lo + hi) / 2; lo <= hi; guess = (lo + hi) / 2) { // Restore styles and colors. They will be truncated to size by SetText. render_text->styles_ = styles_; @@ -1192,12 +1194,10 @@ base::string16 RenderText::ElideText(const base::string16& text) { // whole text. Since we want ellipsis to indicate continuation of the // preceding text, we force the directionality of ellipsis to be same as // the preceding text using LTR or RTL markers. - base::i18n::TextDirection leading_text_direction = - base::i18n::GetFirstStrongCharacterDirection(new_text); base::i18n::TextDirection trailing_text_direction = base::i18n::GetLastStrongCharacterDirection(new_text); new_text.append(ellipsis); - if (trailing_text_direction != leading_text_direction) { + if (trailing_text_direction != text_direction) { if (trailing_text_direction == base::i18n::LEFT_TO_RIGHT) new_text += base::i18n::kLeftToRightMark; else diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h index af5ce30..7af9743 100644 --- a/ui/gfx/render_text.h +++ b/ui/gfx/render_text.h @@ -398,6 +398,12 @@ class GFX_EXPORT RenderText { // chosen. virtual std::vector<FontSpan> GetFontSpansForTesting() = 0; + // Gets the horizontal bounds (relative to the left of the text, not the view) + // of the glyph starting at |index|. If the glyph is RTL then the returned + // Range will have is_reversed() true. (This does not return a Rect because a + // Rect can't have a negative width.) + virtual Range GetGlyphBounds(size_t index) = 0; + protected: RenderText(); @@ -462,12 +468,6 @@ class GFX_EXPORT RenderText { // Sets the selection model, the argument is assumed to be valid. virtual void SetSelectionModel(const SelectionModel& model); - // Get the horizontal bounds (relative to the left of the text, not the view) - // of the glyph starting at |index|. If the glyph is RTL then the returned - // Range will have is_reversed() true. (This does not return a Rect because a - // Rect can't have a negative width.) - virtual Range GetGlyphBounds(size_t index) = 0; - // Get the visual bounds containing the logical substring within the |range|. // If |range| is empty, the result is empty. These bounds could be visually // discontinuous if the substring is split by a LTR/RTL level change. diff --git a/ui/gfx/render_text_pango.cc b/ui/gfx/render_text_pango.cc index f7a61b17..4bfdaa7 100644 --- a/ui/gfx/render_text_pango.cc +++ b/ui/gfx/render_text_pango.cc @@ -211,6 +211,7 @@ SelectionModel RenderTextPango::AdjacentWordSelectionModel( } Range RenderTextPango::GetGlyphBounds(size_t index) { + EnsureLayout(); PangoRectangle pos; pango_layout_index_to_pos(layout_, TextIndexToLayoutIndex(index), &pos); // TODO(derat): Support fractional ranges for subpixel positioning? diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc index 7565921..977cea4 100644 --- a/ui/gfx/render_text_unittest.cc +++ b/ui/gfx/render_text_unittest.cc @@ -419,6 +419,8 @@ TEST_F(RenderTextTest, ElidedText) { } cases[] = { // Strings shorter than the elision width should be laid out in full. { L"", L"" , false }, + { L"M", L"" , false }, + { L" . ", L" . " , false }, { kWeak, kWeak , false }, { kLtr, kLtr , false }, { kLtrRtl, kLtrRtl , false }, @@ -443,7 +445,7 @@ TEST_F(RenderTextTest, ElidedText) { { L"0\x05e9\x05bc\x05c1\x05b8", L"0\x05e9\x05bc\x05c1\x05b8", false }, { L"0\x05e9\x05bc\x05c1\x05b8", L"0\x05e9\x05bc\x2026" , true }, { L"01\x05e9\x05bc\x05c1\x05b8", L"01\x05e9\x2026" , true }, - { L"012\x05e9\x05bc\x05c1\x05b8", L"012\x2026" , true }, + { L"012\x05e9\x05bc\x05c1\x05b8", L"012\x2026\x200E" , true }, { L"012\xF0\x9D\x84\x9E", L"012\xF0\x2026" , true }, }; @@ -468,7 +470,8 @@ TEST_F(RenderTextTest, ElidedText) { render_text->SetText(input); render_text->SetDisplayRect(gfx::Rect(0, 0, expected_width, 100)); - EXPECT_EQ(input, render_text->text()); + EXPECT_EQ(input, render_text->text()) + << "->For case " << i << ": " << cases[i].text << "\n"; EXPECT_EQ(WideToUTF16(cases[i].layout_text), render_text->GetLayoutText()) << "->For case " << i << ": " << cases[i].text << "\n"; expected_render_text->SetText(base::string16()); |