Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(204)

Unified Diff: chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc

Issue 22679003: InstantExtended(gtk): Hide top match if told to so. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Skip DCHECK in test. Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc
diff --git a/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc b/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc
index c0f6dc22f0c2f640bda6f1f01fb952be117a6dca..9d845974739a76462c750e511e4440b1700dfed3 100644
--- a/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc
+++ b/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc
@@ -97,15 +97,6 @@ gfx::Rect GetWindowRect(GdkWindow* window) {
return gfx::Rect(width, height);
}
-// Return a Rect for the space for a result line. This excludes the border,
-// but includes the padding. This is the area that is colored for a selection.
-gfx::Rect GetRectForLine(size_t line, int width) {
- return gfx::Rect(kBorderThickness,
- (line * kHeightPerResult) + kBorderThickness,
- width - (kBorderThickness * 2),
- kHeightPerResult);
-}
-
// TODO(deanm): Find some better home for this, and make it more efficient.
size_t GetUTF8Offset(const string16& text, size_t text_offset) {
return UTF16ToUTF8(text.substr(0, text_offset)).length();
@@ -169,112 +160,26 @@ GdkColor SelectedURLColor(GdkColor foreground, GdkColor background) {
}
} // namespace
-void OmniboxPopupViewGtk::SetupLayoutForMatch(
- PangoLayout* layout,
- const string16& text,
- const AutocompleteMatch::ACMatchClassifications& classifications,
- const GdkColor* base_color,
- const GdkColor* dim_color,
- const GdkColor* url_color,
- const std::string& prefix_text) {
- // In RTL, mark text with left-to-right embedding mark if there is no strong
- // RTL characters inside it, so the ending punctuation displays correctly
- // and the eliding ellipsis displays correctly. We only mark the text with
- // LRE. Wrapping it with LRE and PDF by calling AdjustStringForLocaleDirection
- // or WrapStringWithLTRFormatting will render the elllipsis at the left of the
- // elided pure LTR text.
- bool marked_with_lre = false;
- string16 localized_text = text;
- // Pango is really easy to overflow and send into a computational death
- // spiral that can corrupt the screen. Assume that we'll never have more than
- // 2000 characters, which should be a safe assumption until we all get robot
- // eyes. http://crbug.com/66576
- if (localized_text.length() > 2000)
- localized_text = localized_text.substr(0, 2000);
- bool is_rtl = base::i18n::IsRTL();
- if (is_rtl && !base::i18n::StringContainsStrongRTLChars(localized_text)) {
- localized_text.insert(0, 1, base::i18n::kLeftToRightEmbeddingMark);
- marked_with_lre = true;
- }
-
- // We can have a prefix, or insert additional characters while processing the
- // classifications. We need to take this in to account when we translate the
- // UTF-16 offsets in the classification into text_utf8 byte offsets.
- size_t additional_offset = prefix_text.length(); // Length in utf-8 bytes.
- std::string text_utf8 = prefix_text + UTF16ToUTF8(localized_text);
-
- PangoAttrList* attrs = pango_attr_list_new();
-
- // TODO(deanm): This is a hack, just to handle coloring prefix_text.
- // Hopefully I can clean up the match situation a bit and this will
- // come out cleaner. For now, apply the base color to the whole text
- // so that our prefix will have the base color applied.
- PangoAttribute* base_fg_attr = pango_attr_foreground_new(
- base_color->red, base_color->green, base_color->blue);
- pango_attr_list_insert(attrs, base_fg_attr); // Ownership taken.
-
- // Walk through the classifications, they are linear, in order, and should
- // cover the entire text. We create a bunch of overlapping attributes,
- // extending from the offset to the end of the string. The ones created
- // later will override the previous ones, meaning we will still setup each
- // portion correctly, we just don't need to compute the end offset.
- for (ACMatchClassifications::const_iterator i = classifications.begin();
- i != classifications.end(); ++i) {
- size_t offset = GetUTF8Offset(localized_text, i->offset) +
- additional_offset;
-
- // TODO(deanm): All the colors should probably blend based on whether this
- // result is selected or not. This would include the green URLs. Right
- // now the caller is left to blend only the base color. Do we need to
- // handle things like DIM urls? Turns out DIM means something different
- // than you'd think, all of the description text is not DIM, it is a
- // special case that is not very common, but we should figure out and
- // support it.
- const GdkColor* color = base_color;
- if (i->style & ACMatchClassification::URL) {
- color = url_color;
- // Insert a left to right embedding to make sure that URLs are shown LTR.
- if (is_rtl && !marked_with_lre) {
- std::string lre(kLRE);
- text_utf8.insert(offset, lre);
- additional_offset += lre.length();
- }
- }
-
- if (i->style & ACMatchClassification::DIM)
- color = dim_color;
-
- PangoAttribute* fg_attr = pango_attr_foreground_new(
- color->red, color->green, color->blue);
- fg_attr->start_index = offset;
- pango_attr_list_insert(attrs, fg_attr); // Ownership taken.
-
- // Matched portions are bold, otherwise use the normal weight.
- PangoWeight weight = (i->style & ACMatchClassification::MATCH) ?
- PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
- PangoAttribute* weight_attr = pango_attr_weight_new(weight);
- weight_attr->start_index = offset;
- pango_attr_list_insert(attrs, weight_attr); // Ownership taken.
- }
-
- pango_layout_set_text(layout, text_utf8.data(), text_utf8.length());
- pango_layout_set_attributes(layout, attrs); // Ref taken.
- pango_attr_list_unref(attrs);
-}
-
OmniboxPopupViewGtk::OmniboxPopupViewGtk(const gfx::Font& font,
OmniboxView* omnibox_view,
OmniboxEditModel* edit_model,
GtkWidget* location_bar)
- : model_(new OmniboxPopupModel(this, edit_model)),
- omnibox_view_(omnibox_view),
+ : omnibox_view_(omnibox_view),
location_bar_(location_bar),
window_(gtk_window_new(GTK_WINDOW_POPUP)),
layout_(NULL),
- theme_service_(GtkThemeService::GetFrom(edit_model->profile())),
+ theme_service_(NULL),
font_(font.DeriveFont(kEditFontAdjust)),
ignore_mouse_drag_(false),
opened_(false) {
+ // edit_model may be NULL in unit tests.
+ if (edit_model) {
+ model_.reset(new OmniboxPopupModel(this, edit_model));
+ theme_service_ = GtkThemeService::GetFrom(edit_model->profile());
+ }
+}
+
+void OmniboxPopupViewGtk::Init() {
gtk_widget_set_can_focus(window_, FALSE);
// Don't allow the window to be resized. This also forces the window to
// shrink down to the size of its child contents.
@@ -328,8 +233,11 @@ OmniboxPopupViewGtk::~OmniboxPopupViewGtk() {
// This is because the model destructor can call back into us, and we need
// to make sure everything is still valid when it does.
model_.reset();
- g_object_unref(layout_);
- gtk_widget_destroy(window_);
+ // layout_ may be NULL in unit tests.
+ if (layout_) {
+ g_object_unref(layout_);
+ gtk_widget_destroy(window_);
+ }
}
bool OmniboxPopupViewGtk::IsOpen() const {
@@ -337,6 +245,8 @@ bool OmniboxPopupViewGtk::IsOpen() const {
}
void OmniboxPopupViewGtk::InvalidateLine(size_t line) {
+ if (line < GetHiddenMatchCount())
+ return;
// TODO(deanm): Is it possible to use some constant for the width, instead
// of having to query the width of the window?
GdkWindow* gdk_window = gtk_widget_get_window(GTK_WIDGET(window_));
@@ -346,13 +256,14 @@ void OmniboxPopupViewGtk::InvalidateLine(size_t line) {
}
void OmniboxPopupViewGtk::UpdatePopupAppearance() {
- const AutocompleteResult& result = model_->result();
- if (result.empty()) {
+ const AutocompleteResult& result = GetResult();
+ const size_t hidden_matches = GetHiddenMatchCount();
+ if (result.size() <= hidden_matches) {
Hide();
return;
}
- Show(result.size());
+ Show(result.size() - hidden_matches);
gtk_widget_queue_draw(window_);
}
@@ -427,6 +338,124 @@ void OmniboxPopupViewGtk::Observe(int type,
gtk_widget_modify_bg(window_, GTK_STATE_NORMAL, &background_color_);
}
+size_t OmniboxPopupViewGtk::LineFromY(int y) const {
+ // model_ may be NULL in unit tests.
+ if (model_)
+ DCHECK_NE(0U, model_->result().size());
+ size_t line = std::max(y - kBorderThickness, 0) / kHeightPerResult;
+ return std::min(line + GetHiddenMatchCount(), GetResult().size() - 1);
+}
+
+gfx::Rect OmniboxPopupViewGtk::GetRectForLine(size_t line, int width) const {
+ size_t visible_line = line - GetHiddenMatchCount();
+ return gfx::Rect(kBorderThickness,
+ (visible_line * kHeightPerResult) + kBorderThickness,
+ width - (kBorderThickness * 2),
+ kHeightPerResult);
+}
+
+size_t OmniboxPopupViewGtk::GetHiddenMatchCount() const {
+ return GetResult().ShouldHideTopMatch() ? 1 : 0;
+}
+
+const AutocompleteResult& OmniboxPopupViewGtk::GetResult() const {
+ return model_->result();
+}
+
+// static
+void OmniboxPopupViewGtk::SetupLayoutForMatch(
+ PangoLayout* layout,
+ const string16& text,
+ const AutocompleteMatch::ACMatchClassifications& classifications,
+ const GdkColor* base_color,
+ const GdkColor* dim_color,
+ const GdkColor* url_color,
+ const std::string& prefix_text) {
+ // In RTL, mark text with left-to-right embedding mark if there is no strong
+ // RTL characters inside it, so the ending punctuation displays correctly
+ // and the eliding ellipsis displays correctly. We only mark the text with
+ // LRE. Wrapping it with LRE and PDF by calling AdjustStringForLocaleDirection
+ // or WrapStringWithLTRFormatting will render the elllipsis at the left of the
+ // elided pure LTR text.
+ bool marked_with_lre = false;
+ string16 localized_text = text;
+ // Pango is really easy to overflow and send into a computational death
+ // spiral that can corrupt the screen. Assume that we'll never have more than
+ // 2000 characters, which should be a safe assumption until we all get robot
+ // eyes. http://crbug.com/66576
+ if (localized_text.length() > 2000)
+ localized_text = localized_text.substr(0, 2000);
+ bool is_rtl = base::i18n::IsRTL();
+ if (is_rtl && !base::i18n::StringContainsStrongRTLChars(localized_text)) {
+ localized_text.insert(0, 1, base::i18n::kLeftToRightEmbeddingMark);
+ marked_with_lre = true;
+ }
+
+ // We can have a prefix, or insert additional characters while processing the
+ // classifications. We need to take this in to account when we translate the
+ // UTF-16 offsets in the classification into text_utf8 byte offsets.
+ size_t additional_offset = prefix_text.length(); // Length in utf-8 bytes.
+ std::string text_utf8 = prefix_text + UTF16ToUTF8(localized_text);
+
+ PangoAttrList* attrs = pango_attr_list_new();
+
+ // TODO(deanm): This is a hack, just to handle coloring prefix_text.
+ // Hopefully I can clean up the match situation a bit and this will
+ // come out cleaner. For now, apply the base color to the whole text
+ // so that our prefix will have the base color applied.
+ PangoAttribute* base_fg_attr = pango_attr_foreground_new(
+ base_color->red, base_color->green, base_color->blue);
+ pango_attr_list_insert(attrs, base_fg_attr); // Ownership taken.
+
+ // Walk through the classifications, they are linear, in order, and should
+ // cover the entire text. We create a bunch of overlapping attributes,
+ // extending from the offset to the end of the string. The ones created
+ // later will override the previous ones, meaning we will still setup each
+ // portion correctly, we just don't need to compute the end offset.
+ for (ACMatchClassifications::const_iterator i = classifications.begin();
+ i != classifications.end(); ++i) {
+ size_t offset = GetUTF8Offset(localized_text, i->offset) +
+ additional_offset;
+
+ // TODO(deanm): All the colors should probably blend based on whether this
+ // result is selected or not. This would include the green URLs. Right
+ // now the caller is left to blend only the base color. Do we need to
+ // handle things like DIM urls? Turns out DIM means something different
+ // than you'd think, all of the description text is not DIM, it is a
+ // special case that is not very common, but we should figure out and
+ // support it.
+ const GdkColor* color = base_color;
+ if (i->style & ACMatchClassification::URL) {
+ color = url_color;
+ // Insert a left to right embedding to make sure that URLs are shown LTR.
+ if (is_rtl && !marked_with_lre) {
+ std::string lre(kLRE);
+ text_utf8.insert(offset, lre);
+ additional_offset += lre.length();
+ }
+ }
+
+ if (i->style & ACMatchClassification::DIM)
+ color = dim_color;
+
+ PangoAttribute* fg_attr = pango_attr_foreground_new(
+ color->red, color->green, color->blue);
+ fg_attr->start_index = offset;
+ pango_attr_list_insert(attrs, fg_attr); // Ownership taken.
+
+ // Matched portions are bold, otherwise use the normal weight.
+ PangoWeight weight = (i->style & ACMatchClassification::MATCH) ?
+ PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
+ PangoAttribute* weight_attr = pango_attr_weight_new(weight);
+ weight_attr->start_index = offset;
+ pango_attr_list_insert(attrs, weight_attr); // Ownership taken.
+ }
+
+ pango_layout_set_text(layout, text_utf8.data(), text_utf8.length());
+ pango_layout_set_attributes(layout, attrs); // Ref taken.
+ pango_attr_list_unref(attrs);
+}
+
void OmniboxPopupViewGtk::Show(size_t num_results) {
gint origin_x, origin_y;
GdkWindow* gdk_window = gtk_widget_get_window(location_bar_);
@@ -460,18 +489,12 @@ void OmniboxPopupViewGtk::StackWindow() {
ui::StackPopupWindow(window_, toplevel);
}
-size_t OmniboxPopupViewGtk::LineFromY(int y) {
- DCHECK_NE(0U, model_->result().size());
- size_t line = std::max(y - kBorderThickness, 0) / kHeightPerResult;
- return std::min(line, model_->result().size() - 1);
-}
-
void OmniboxPopupViewGtk::AcceptLine(size_t line,
WindowOpenDisposition disposition) {
// OpenMatch() may close the popup, which will clear the result set and, by
// extension, |match| and its contents. So copy the relevant match out to
// make sure it stays alive until the call completes.
- AutocompleteMatch match = model_->result().match_at(line);
+ AutocompleteMatch match = GetResult().match_at(line);
omnibox_view_->OpenMatch(match, disposition, GURL(), line);
}
@@ -521,7 +544,7 @@ void OmniboxPopupViewGtk::GetVisibleMatchForInput(
size_t index,
const AutocompleteMatch** match,
bool* is_selected_keyword) {
- const AutocompleteResult& result = model_->result();
+ const AutocompleteResult& result = GetResult();
if (result.match_at(index).associated_keyword.get() &&
model_->selected_line() == index &&
@@ -594,7 +617,7 @@ gboolean OmniboxPopupViewGtk::HandleButtonRelease(GtkWidget* widget,
gboolean OmniboxPopupViewGtk::HandleExpose(GtkWidget* widget,
GdkEventExpose* event) {
bool ltr = !base::i18n::IsRTL();
- const AutocompleteResult& result = model_->result();
+ const AutocompleteResult& result = GetResult();
gfx::Rect window_rect = GetWindowRect(event->window);
gfx::Rect damage_rect = gfx::Rect(event->area);
@@ -621,7 +644,7 @@ gboolean OmniboxPopupViewGtk::HandleExpose(GtkWidget* widget,
pango_layout_set_height(layout_, kHeightPerResult * PANGO_SCALE);
- for (size_t i = 0; i < result.size(); ++i) {
+ for (size_t i = GetHiddenMatchCount(); i < result.size(); ++i) {
gfx::Rect line_rect = GetRectForLine(i, window_rect.width());
// Only repaint and layout damaged lines.
if (!line_rect.Intersects(damage_rect))

Powered by Google App Engine
This is Rietveld 408576698