OmniboxPopupViewMac refactoring Part 3 (truncation)

This CL changes how omnibox popup results are truncated. Previously content
text would be truncated at 70% which might leave empty space if the description
was short. I've made the following changes to the truncation code:
  - move the truncation logic to the button cell
  - have the content text fill up unused space when truncating
  - use a fade effect to truncate instead of ellipses

BUG=9977, 252988

Review URL: https://chromiumcodereview.appspot.com/17774002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@217109 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h

@@ -7,12 +7,17 @@
 
 #import <Cocoa/Cocoa.h>
 
-// OmniboxPopupCell overrides how backgrounds are displayed to
-// handle hover versus selected.  So long as we're in there, it also
-// provides some default initialization.
+#include "base/mac/scoped_nsobject.h"
+
+// Draws a single row in the omnibox popup.
 @interface OmniboxPopupCell : NSButtonCell {
+  base::scoped_nsobject<NSAttributedString> contentText_;
+  base::scoped_nsobject<NSAttributedString> descriptionText_;
 }
 
+- (void)setContentText:(NSAttributedString*)contentText;
+- (void)setDescriptionText:(NSAttributedString*)descriptionText;
+
 @end
 
 #endif  // CHROME_BROWSER_UI_COCOA_OMNIBOX_OMNIBOX_POPUP_CELL_H_

chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm

@@ -4,8 +4,11 @@
 
 #import "chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h"
 
+#include <algorithm>
 #include <cmath>
 
+#include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
+
 namespace {
 
 // How far to offset image column from the left.
@@ -14,14 +17,45 @@ const CGFloat kImageXOffset = 5.0;
 // How far to offset the text column from the left.
 const CGFloat kTextXOffset = 28.0;
 
+// Maximum fraction of the popup width that can be used to display match
+// contents.
+const CGFloat kMinDescriptionFraction = 0.7;
+
 // Rounding radius of selection and hover background on popup items.
 const CGFloat kCellRoundingRadius = 2.0;
 
-NSColor* SelectedBackgroundColor() {
-  return [NSColor selectedControlColor];
-}
-NSColor* HoveredBackgroundColor() {
-  return [NSColor controlHighlightColor];
+void DrawFadeTruncatingTitle(NSAttributedString* title,
+                             NSRect titleRect,
+                             NSColor* backgroundColor) {
+  gfx::ScopedNSGraphicsContextSaveGState scopedGState;
+  NSRectClip(titleRect);
+
+  // Draw the entire text.
+  NSSize textSize = [title size];
+  NSPoint textOrigin = titleRect.origin;
+  textOrigin.y += roundf((NSHeight(titleRect) - textSize.height) / 2.0) - 1.0;
+  [title drawAtPoint:textOrigin];
+
+  // Empirically, Cocoa will draw an extra 2 pixels past NSWidth(titleRect)
+  // before it clips the text.
+  const CGFloat kOverflowBeforeClip = 2.0;
+  CGFloat clipped_width = NSWidth(titleRect) + kOverflowBeforeClip;
+  if (textSize.width <= clipped_width)
+    return;
+
+  // The gradient width is the same as the line height.
+  CGFloat gradientWidth = std::min(textSize.height, NSWidth(titleRect) / 4);
+
+  // Draw the gradient part.
+  NSColor *alphaColor = [backgroundColor colorWithAlphaComponent:0.0];
+  base::scoped_nsobject<NSGradient> mask(
+      [[NSGradient alloc] initWithStartingColor:alphaColor
+                                    endingColor:backgroundColor]);
+  [mask drawFromPoint:NSMakePoint(NSMaxX(titleRect) - gradientWidth,
+                                  NSMinY(titleRect))
+              toPoint:NSMakePoint(NSMaxX(titleRect),
+                                  NSMinY(titleRect))
+              options:NSGradientDrawsBeforeStartingLocation];
 }
 
 }  // namespace
@@ -29,8 +63,7 @@ NSColor* HoveredBackgroundColor() {
 @implementation OmniboxPopupCell
 
 - (id)init {
-  self = [super init];
-  if (self) {
+  if ((self = [super init])) {
     [self setImagePosition:NSImageLeft];
     [self setBordered:NO];
     [self setButtonType:NSRadioButton];
@@ -41,15 +74,28 @@ NSColor* HoveredBackgroundColor() {
   return self;
 }
 
-// The default NSButtonCell drawing leaves the image flush left and
-// the title next to the image.  This spaces things out to line up
-// with the star button and autocomplete field.
+- (void)setContentText:(NSAttributedString*)contentText {
+  contentText_.reset([contentText retain]);
+}
+
+- (void)setDescriptionText:(NSAttributedString*)descriptionText {
+  base::scoped_nsobject<NSMutableAttributedString> dashDescriptionText(
+      [[NSMutableAttributedString alloc]
+          initWithAttributedString:descriptionText]);
+  NSString* rawEnDash = @" \u2013 ";
+  [dashDescriptionText replaceCharactersInRange:NSMakeRange(0, 0)
+                                     withString:rawEnDash];
+  descriptionText_.reset(dashDescriptionText.release());
+}
+
 - (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView *)controlView {
+  NSColor* backgroundColor = [NSColor controlBackgroundColor];
   if ([self state] == NSOnState || [self isHighlighted]) {
     if ([self state] == NSOnState)
-      [SelectedBackgroundColor() set];
+      backgroundColor = [NSColor selectedControlColor];
     else
-      [HoveredBackgroundColor() set];
+      backgroundColor = [NSColor controlHighlightColor];
+    [backgroundColor set];
     NSBezierPath* path =
         [NSBezierPath bezierPathWithRoundedRect:cellFrame
                                         xRadius:kCellRoundingRadius
@@ -74,12 +120,27 @@ NSColor* HoveredBackgroundColor() {
   }
 
   // Adjust the title position to be lined up under the field's text.
-  NSAttributedString* title = [self attributedTitle];
-  if (title && [title length]) {
-    NSRect titleRect = cellFrame;
-    titleRect.size.width -= kTextXOffset;
-    titleRect.origin.x += kTextXOffset;
-    [self drawTitle:title withFrame:titleRect inView:controlView];
+  if ([contentText_ length]) {
+    NSRect availRect = cellFrame;
+    availRect.size.width = NSWidth(cellFrame) - kTextXOffset;
+    availRect.origin.x += kTextXOffset;
+    CGFloat availWidth = NSWidth(availRect);
+    CGFloat contentWidth = [contentText_ size].width;
+    CGFloat descWidth =
+        [descriptionText_ length] ? [descriptionText_ size].width : 0;
+
+    CGFloat tempDescWidth =
+        std::min(descWidth, kMinDescriptionFraction * availWidth);
+    contentWidth = std::min(contentWidth, availWidth - tempDescWidth);
+
+    NSRect contentRect;
+    NSRect descRect;
+    NSDivideRect(
+        availRect, &contentRect, &descRect, contentWidth, NSMinXEdge);
+
+    DrawFadeTruncatingTitle(contentText_, contentRect, backgroundColor);
+    if ([descriptionText_ length])
+      DrawFadeTruncatingTitle(descriptionText_, descRect, backgroundColor);
   }
 }
 

chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h

@@ -50,13 +50,6 @@ class OmniboxPopupViewMac : public OmniboxPopupView,
 
   OmniboxPopupMatrix* matrix() { return matrix_; }
 
-  // Return the text to show for the match, based on the match's
-  // contents and description.  Result will be in |font|, with the
-  // boldfaced version used for matches.
-  static NSAttributedString* MatchText(const AutocompleteMatch& match,
-                                       gfx::Font& font,
-                                       float cell_width);
-
   // Applies the given font and colors to the match string based on
   // classifications.
   static NSMutableAttributedString* DecorateMatchedString(
@@ -66,18 +59,6 @@ class OmniboxPopupViewMac : public OmniboxPopupView,
       NSColor* dim_text_color,
       gfx::Font& font);
 
-  // Helper for MatchText() to elide a marked-up string using
-  // gfx::ElideText() as a model.  Modifies |a_string| in place.
-  // TODO(shess): Consider breaking AutocompleteButtonCell out of this
-  // code, and modifying it to have something like -setMatch:, so that
-  // these convolutions to expose internals for testing can be
-  // cleaner.
-  static NSMutableAttributedString* ElideString(
-      NSMutableAttributedString* a_string,
-      const string16& original_string,
-      const gfx::Font& font,
-      const float cell_width);
-
  protected:
   // Gets the autocomplete results. This is virtual so that it can be overriden
   // by tests.

chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

@@ -24,9 +24,7 @@
 #import "ui/base/cocoa/flipped_view.h"
 #include "ui/base/cocoa/window_size_constants.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/base/text/text_elider.h"
 #include "ui/gfx/rect.h"
-#include "ui/gfx/scoped_ns_graphics_context_save_gstate_mac.h"
 
 namespace {
 
@@ -37,16 +35,9 @@ const CGFloat kCellHeightAdjust = 6.0;
 // Padding between matrix and the top and bottom of the popup window.
 const CGFloat kPopupPaddingVertical = 5.0;
 
-// How far to offset the text column from the left.
-const CGFloat kTextXOffset = 28.0;
-
 // Animation duration when animating the popup window smaller.
 const NSTimeInterval kShrinkAnimationDuration = 0.1;
 
-// Maximum fraction of the popup width that can be used to display match
-// contents.
-const CGFloat kMaxContentsFraction = 0.7;
-
 // Background colors for different states of the popup elements.
 NSColor* BackgroundColor() {
   return [NSColor controlBackgroundColor];
@@ -127,7 +118,20 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
     OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
     const AutocompleteMatch& match = GetResult().match_at(ii + start_match);
     [cell setImage:ImageForMatch(match)];
-    [cell setAttributedTitle:MatchText(match, result_font, matrix_width)];
+    [cell setContentText:DecorateMatchedString(match.contents,
+                                               match.contents_class,
+                                               ContentTextColor(),
+                                               DimContentTextColor(),
+                                               result_font)];
+    if (match.description.empty()) {
+      [cell setDescriptionText:nil];
+    } else {
+      [cell setDescriptionText:DecorateMatchedString(match.description,
+                                                     match.description_class,
+                                                     DimContentTextColor(),
+                                                     DimContentTextColor(),
+                                                     result_font)];
+    }
   }
 
   // Set the cell size to fit a line of text in the cell's font.  All
@@ -185,64 +189,6 @@ void OmniboxPopupViewMac::OnMatrixRowMiddleClicked(OmniboxPopupMatrix* matrix,
   OpenURLForRow(row, NEW_BACKGROUND_TAB);
 }
 
-// Return the text to show for the match, based on the match's
-// contents and description.  Result will be in |font|, with the
-// boldfaced version used for matches.
-NSAttributedString* OmniboxPopupViewMac::MatchText(
-    const AutocompleteMatch& match,
-    gfx::Font& font,
-    float cell_width) {
-  NSMutableAttributedString *as =
-      DecorateMatchedString(match.contents,
-                            match.contents_class,
-                            ContentTextColor(),
-                            DimContentTextColor(),
-                            font);
-
-  // If there is a description, append it, separated from the contents
-  // with an en dash, and decorated with a distinct color.
-  if (!match.description.empty()) {
-    // Make sure the current string fits w/in kMaxContentsFraction of
-    // the cell to make sure the description will be at least
-    // partially visible.
-    // TODO(shess): Consider revising our NSCell subclass to have two
-    // bits and just draw them right, rather than truncating here.
-    const float text_width = cell_width - kTextXOffset;
-    as = ElideString(as, match.contents, font,
-                     text_width * kMaxContentsFraction);
-
-    NSDictionary* attributes = @{
-        NSFontAttributeName :  font.GetNativeFont(),
-        NSForegroundColorAttributeName : ContentTextColor()
-    };
-    NSString* raw_en_dash = @" \u2013 ";
-    NSAttributedString* en_dash =
-        [[[NSAttributedString alloc] initWithString:raw_en_dash
-                                         attributes:attributes] autorelease];
-
-    // In Windows, a boolean force_dim is passed as true for the
-    // description.  Here, we pass the dim text color for both normal and dim,
-    // to accomplish the same thing.
-    NSAttributedString* description =
-        DecorateMatchedString(match.description, match.description_class,
-                              DimContentTextColor(),
-                              DimContentTextColor(),
-                              font);
-
-    [as appendAttributedString:en_dash];
-    [as appendAttributedString:description];
-  }
-
-  NSMutableParagraphStyle* style =
-      [[[NSMutableParagraphStyle alloc] init] autorelease];
-  [style setLineBreakMode:NSLineBreakByTruncatingTail];
-  [style setTighteningFactorForTruncation:0.0];
-  [as addAttribute:NSParagraphStyleAttributeName value:style
-             range:NSMakeRange(0, [as length])];
-
-  return as;
-}
-
 // static
 NSMutableAttributedString* OmniboxPopupViewMac::DecorateMatchedString(
     const string16& match_string,
@@ -308,42 +254,6 @@ NSMutableAttributedString* OmniboxPopupViewMac::DecorateMatchedString(
   return as;
 }
 
-NSMutableAttributedString* OmniboxPopupViewMac::ElideString(
-    NSMutableAttributedString* a_string,
-    const string16& original_string,
-    const gfx::Font& font,
-    const float width) {
-  // If it already fits, nothing to be done.
-  if ([a_string size].width <= width) {
-    return a_string;
-  }
-
-  // If ElideText() decides to do nothing, nothing to be done.
-  const string16 elided =
-      ui::ElideText(original_string, font, width, ui::ELIDE_AT_END);
-  if (0 == elided.compare(original_string)) {
-    return a_string;
-  }
-
-  // If everything was elided away, clear the string.
-  if (elided.empty()) {
-    [a_string deleteCharactersInRange:NSMakeRange(0, [a_string length])];
-    return a_string;
-  }
-
-  // The ellipses should be the last character, and everything before
-  // that should match the original string.
-  const size_t i(elided.length() - 1);
-  DCHECK_NE(0, elided.compare(0, i, original_string));
-
-  // Replace the end of |aString| with the ellipses from |elided|.
-  NSString* s = base::SysUTF16ToNSString(elided.substr(i));
-  [a_string replaceCharactersInRange:NSMakeRange(i, [a_string length] - i)
-                          withString:s];
-
-  return a_string;
-}
-
 const AutocompleteResult& OmniboxPopupViewMac::GetResult() const {
   return model_->result();
 }