Elide text in the new Autofill UI

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

Index: chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
index 6473184eadf65474d343151d4f7d8ea8de7e3194..ca3a636ede29f6bb6073b0b19c957c316b94ee37 100644
--- a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
@@ -12,6 +12,10 @@
 #include "grit/webkit_resources.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h"
 #include "ui/base/events/event.h"
+#include "ui/base/text/text_elider.h"
+#include "ui/gfx/display.h"
+#include "ui/gfx/screen.h"
+#include "ui/gfx/vector2d.h"
 
 using WebKit::WebAutofillClient;
 
@@ -118,12 +122,34 @@ void AutofillPopupControllerImpl::Show(
 #if !defined(OS_ANDROID)
   // Android displays the long text with ellipsis using the view attributes.
 
-  // TODO(csharp): Fix crbug.com/156163 and use better logic when clipping.
+  int popup_width = CalculateAndSetPopupWidth();
+
+  // Elide the name and subtext strings so that the popup fits in the available
+  // space.
   for (size_t i = 0; i < names_.size(); ++i) {
-    if (names_[i].length() > 15)
-      names_[i].erase(15);
-    if (subtexts[i].length() > 15)
-      subtexts_[i].erase(15);
+    int total_text_length = name_font().GetStringWidth(names[i]) +
+        subtext_font().GetStringWidth(subtexts_[i]);
+    // The line can have no strings if it represents a UI element, such as
+    // a separator line.
+    if (total_text_length == 0)
+      continue;
+
+    int available_width = popup_width - RowWidthWithoutText(i);
+
+    // Each field recieves space in proportion to its length.
+    int name_size = available_width *
+        name_font().GetStringWidth(names_[i]) / total_text_length;

[Ilya Sherman, 2013/01/11 23:05:35]

nit: Might be worth caching the result of the GetStringWidth() calls within this
loop (i.e. from lines 130-131), as it's probably one of the more expensive
computations in this class.

[csharp, 2013/01/14 20:32:33]

Done.

+    names_[i] = ui::ElideText(names_[i],
+                              name_font(),
+                              name_size,
+                              ui::ELIDE_AT_END);
+
+    int subtext_size = available_width *
+        subtext_font().GetStringWidth(subtexts[i]) / total_text_length;
+    subtexts_[i] = ui::ElideText(subtexts_[i],
+                                 subtext_font(),
+                                 subtext_size,
+                                 ui::ELIDE_AT_END);
   }
 #endif
 
@@ -209,21 +235,9 @@ int AutofillPopupControllerImpl::GetPopupRequiredWidth() {
   int popup_width = element_bounds().width();
   DCHECK_EQ(names().size(), subtexts().size());
   for (size_t i = 0; i < names().size(); ++i) {
-    int row_size = kEndPadding +
-        name_font_.GetStringWidth(names()[i]) +
-        kNamePadding +
-        subtext_font_.GetStringWidth(subtexts()[i]);
-
-    // Add the Autofill icon size, if required.
-    if (!icons()[i].empty())
-      row_size += kAutofillIconWidth + kIconPadding;
-
-    // Add delete icon, if required.
-    if (CanDelete(i))
-      row_size += kDeleteIconWidth + kIconPadding;
-
-    // Add the padding at the end
-    row_size += kEndPadding;
+    int row_size = name_font_.GetStringWidth(names()[i]) +
+        subtext_font_.GetStringWidth(subtexts()[i]) +
+        RowWidthWithoutText(i);
 
     popup_width = std::max(popup_width, row_size);
   }
@@ -504,3 +518,60 @@ void AutofillPopupControllerImpl::ShowView() {
 void AutofillPopupControllerImpl::InvalidateRow(size_t row) {
   view_->InvalidateRow(row);
 }
+
+int AutofillPopupControllerImpl::RowWidthWithoutText(int row) {
+  int row_size = kEndPadding + kNamePadding;
+
+  // Add the Autofill icon size, if required.
+  if (!icons_[row].empty())
+    row_size += kAutofillIconWidth + kIconPadding;
+
+  // Add the delete icon size, if required.
+  if (CanDelete(row))
+    row_size += kDeleteIconWidth + kIconPadding;
+
+  // Add the padding at the end
+  row_size += kEndPadding;
+
+  return row_size;
+}
+
+int AutofillPopupControllerImpl::CalculateAndSetPopupWidth() {
+  int popup_required_width = GetPopupRequiredWidth();
+  gfx::Point right_corner_of_popup = element_bounds().origin() +
+      gfx::Vector2d(popup_required_width, 0);
+
+  gfx::Screen* screen =
+      gfx::Screen::GetScreenFor(container_view());
+  gfx::Display left_display = screen->GetDisplayNearestPoint(
+      element_bounds().origin());

[Ilya Sherman, 2013/01/11 23:05:35]

Hmm, shouldn't this be the display that's at the left corner of the popup were
it to grow from the right corner of the element bounds?

[csharp, 2013/01/14 20:32:33]

Yup, fixed.

+  gfx::Display right_display = screen->GetDisplayNearestPoint(
+      right_corner_of_popup);
+
+  int leftmost_display_x = left_display.bounds().x() *
+      left_display.device_scale_factor();
+  int rightmost_display_x = right_display.GetSizeInPixel().width() +
+      right_display.bounds().x() * right_display.device_scale_factor();
+
+  // Calculate the leftmost and rightmost values the popup can have without
+  // going off the screen.
+  int popup_left_cutoff = std::max(element_bounds().origin().x(),
+                                     leftmost_display_x);

[Ilya Sherman, 2013/01/11 23:05:35]

nit: indentation

[csharp, 2013/01/14 20:32:33]

Done.

+  int popup_right_cutoff = std::min(element_bounds().top_right().x(),
+                                      rightmost_display_x);

[Ilya Sherman, 2013/01/11 23:05:35]

nit: indentation

[csharp, 2013/01/14 20:32:33]

Done.

+
+  int right_available = rightmost_display_x - popup_left_cutoff;
+  int left_available = popup_right_cutoff - leftmost_display_x;
+
+  int popup_width = std::min(popup_required_width,
+                             std::max(right_available, left_available));

[Ilya Sherman, 2013/01/11 23:05:35]

nit: #include <algorithm> for std::min and ::max.

[csharp, 2013/01/14 20:32:33]

Done.

+  // If there is enough space for the popup on the right, show it there,
+  // otherwise choose the larger size.
+  if (right_available >= popup_width || right_available >= left_available) {
+    popup_bounds_.set_x(popup_left_cutoff);
+  } else {
+    popup_bounds_.set_x(popup_right_cutoff - popup_width);
+  }

[Ilya Sherman, 2013/01/11 23:05:35]

nit: No need for curlies

[csharp, 2013/01/14 20:32:33]

Done.

+
+  return popup_width;
+}