Elide text in the new Autofill UI

chrome/browser/ui/autofill/autofill_popup_controller_impl.cc

 // 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.
 
 #include "chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
 
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/ui/autofill/autofill_popup_delegate.h"
 #include "chrome/browser/ui/autofill/autofill_popup_view.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #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;
 
 namespace {
 
 // Used to indicate that no line is currently selected by the user.
 const int kNoSelection = -1;
 
 // Size difference between name and subtext in pixels.
 const int kLabelFontSizeDelta = -2;
@@ skipping 86 matching lines @@
     const std::vector<string16>& icons,
     const std::vector<int>& identifiers) {
   names_ = names;
   subtexts_ = subtexts;
   icons_ = icons;
   identifiers_ = identifiers;
 
 #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
 
   if (!view_) {
     view_ = AutofillPopupView::Create(this);
     ShowView();
   } else {
     UpdateBoundsAndRedrawPopup();
   }
 }
@@ skipping 54 matching lines @@
 }
 
 bool AutofillPopupControllerImpl::CanDelete(size_t index) {
   int id = identifiers_[index];
   return id > 0 ||
       id == WebAutofillClient::MenuItemIDAutocompleteEntry ||
       id == WebAutofillClient::MenuItemIDPasswordEntry;
 }
 
 #if !defined(OS_ANDROID)
 int AutofillPopupControllerImpl::GetPopupRequiredWidth() {

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

Rather than exposing this method to the view, IMO it would be better to just
calculate this as an implementation detail of the controller and update the
popup bounds directly.  Ditto for the height.

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

Done.

   if (name_font_.platform_font() == NULL ||
       subtext_font_.platform_font() == NULL) {
     // We can't calculate the size of the popup if the fonts
     // aren't present.
     return 0;
   }
 
   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);
   }
 
   return popup_width;
 }
 
 int AutofillPopupControllerImpl::GetPopupRequiredHeight() {
   int popup_height = 0;
 
@@ skipping 260 matching lines @@
        identifiers_[0] == WebAutofillClient::MenuItemIDDataListEntry);
 }
 
 void AutofillPopupControllerImpl::ShowView() {
   view_->Show();
 }
 
 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;
+}