aura: Really long tooltips should be wrapped not truncated.

ash/tooltips/tooltip_controller.cc

Index: ash/tooltips/tooltip_controller.cc
diff --git a/ash/tooltips/tooltip_controller.cc b/ash/tooltips/tooltip_controller.cc
index 52049e73f2e05e6a8d96d1d3dab0acbf6d8470b7..6e533c4bd325e9b0ad1379f07f2be7737fc5c24a 100644
--- a/ash/tooltips/tooltip_controller.cc
+++ b/ash/tooltips/tooltip_controller.cc
@@ -33,6 +33,14 @@ const SkColor kTooltipBackground = 0xFFFFFFCC;
 const SkColor kTooltipBorder = 0xFF646450;
 const int kTooltipBorderWidth = 1;
 const int kTooltipHorizontalPadding = 3;
+
+// Max visual tooltip width. If a tooltip is greater than this width, it will
+// be wrapped.
+const int kTooltipMaxWidthPixels = 400;

[sky, 2012/03/21 04:17:23]

On the windows side we initially had tooltips extending the width of the screen.
This seemed problematic so we did something like that. That was even more
problematic so we went back to the width of the screen. You should want to
restrict them?

[varunjain, 2012/03/21 04:48:38]

I restrict them here to the smaller of available screen space and
kTooltipMaxWidthPixels. How is that problematic.
AFAICT, tooltip size is restricted even in render_widget_host_view_win (so
tooltips on webpages on win are restricted).

+
+// Maximum number of lines we allow in the tooltip.
+const size_t kMaxLines = 10;
+
 // TODO(derat): This padding is needed on Chrome OS devices but seems excessive
 // when running the same binary on a Linux workstation; presumably there's a
 // difference in font metrics.  Rationalize this.
@@ -46,9 +54,6 @@ const int kCursorOffsetY = 15;
 // Maximum number of characters we allow in a tooltip.
 const size_t kMaxTooltipLength = 1024;
 
-// Maximum number of lines we allow in the tooltip.
-const size_t kMaxLines = 6;
-
 gfx::Font GetDefaultFont() {
   // TODO(varunjain): implementation duplicated in tooltip_manager_aura. Figure
   // out a way to merge.
@@ -82,26 +87,67 @@ void TrimTooltipToFit(string16* text,
     *text = text->substr(0, kMaxTooltipLength);
 
   // Determine the available width for the tooltip.
-  int available_width = GetMaxWidth(x, y);
+  int available_width = std::min(kTooltipMaxWidthPixels, GetMaxWidth(x, y));
 
-  // Split the string into at most kMaxLines lines.
   std::vector<string16> lines;
   base::SplitString(*text, '\n', &lines);
-  if (lines.size() > kMaxLines)
-    lines.resize(kMaxLines);
-  *line_count = static_cast<int>(lines.size());
+  std::vector<string16> result_lines;
 
   // Format each line to fit.
   gfx::Font font = GetDefaultFont();
+  for (std::vector<string16>::iterator l = lines.begin(); l != lines.end();
+      ++l) {
+    // We break the line at word boundaries, then stuff as many words as we can
+    // in the available width to the current line, and move the remaining words
+    // to a new line.
+    std::vector<string16> words;
+    base::SplitStringDontTrim(*l, ' ', &words);

[Daniel Erat, 2012/03/21 00:55:40]

is it okay to rebuild the string like this, or are we supposed to preserve
whitespace?

[varunjain, 2012/03/21 04:48:38]

whitespace is preserver when I use SplitStringDontTrim. I have also added a
testcase to test this.

+    int current_width = 0;
+    string16 line;
+    for (std::vector<string16>::iterator w = words.begin(); w != words.end();
+        ++w) {
+      string16 word = *w;
+      if (w + 1 != words.end())
+        word.push_back(' ');
+      int word_width = font.GetStringWidth(word);
+      if (current_width + word_width > available_width) {
+        // Current width will exceed the available width. Must start a new line.
+        result_lines.push_back(line);
+        current_width = 0;
+        line.clear();
+      }
+      current_width += word_width;
+      line.append(word);
+    }
+    result_lines.push_back(line);
+  }
+
+  // Clamp number of lines to |kMaxLines|.
+  if (result_lines.size() > kMaxLines) {
+    result_lines.resize(kMaxLines);
+    // Add ellipses character to last line.
+    result_lines[kMaxLines - 1] = ui::TruncateString(
+        result_lines.back(), result_lines.back().length() - 1);
+  }
+  *line_count = result_lines.size();
+
+  // Flatten the result.
   string16 result;
-  for (std::vector<string16>::iterator i = lines.begin(); i != lines.end();
-       ++i) {
-    string16 elided_text =
-        ui::ElideText(*i, font, available_width, ui::ELIDE_AT_END);
-    *max_width = std::max(*max_width, font.GetStringWidth(elided_text));
+  for (std::vector<string16>::iterator l = result_lines.begin();
+      l != result_lines.end(); ++l) {
     if (!result.empty())
       result.push_back('\n');
-    result.append(elided_text);
+    int line_width = font.GetStringWidth(*l);
+    // Since we only break at word boundaries, it could happen that due to some
+    // very long word, line_width is greater than the available_width. In such
+    // case, we simply truncate at available_width and add ellipses at the end.
+    if (line_width > available_width) {
+      *max_width = available_width;
+      result.append(ui::ElideText(*l, font, available_width, ui::ELIDE_AT_END));
+    } else {
+      *max_width = std::max(*max_width, line_width);
+      result.append(*l);
+    }
   }
   *text = result;
 }