Improvements to tab title prefix eliding as per email discussions.

chrome/browser/ui/views/tabs/base_tab.cc

 // Copyright (c) 2011 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/views/tabs/base_tab.h"
 
 #include <limits>
-

[Peter Kasting, 2011/04/01 20:19:20]

Nit: Don't remove this blank line

[MAD, 2011/04/05 16:13:12]

D'Ho...ne. :-)

 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
+#include "chrome/browser/ui/title_prefix_matcher.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/tabs/tab_controller.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "grit/app_resources.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/animation/animation_container.h"
 #include "ui/base/animation/slide_animation.h"
@@ skipping 449 matching lines @@
   const gfx::Rect& title_bounds = GetTitleBounds();
   string16 title = data().title;
   if (title.empty()) {
     title = data().loading ?
         l10n_util::GetStringUTF16(IDS_TAB_LOADING_TITLE) :
         TabContentsWrapper::GetDefaultTitle();
   } else {
     Browser::FormatTitleForDisplay(&title);
     // If we'll need to truncate, check if we should also truncate
     // a common prefix, but only if there is enough room for it.
-    // We arbitrarily choose to request enough room for 10 average chars.
-    if (data().common_prefix_length > 0 &&
-        font_->GetExpectedTextWidth(10) < title_bounds.width() &&
+    // We arbitrarily choose to request enough room for 6 average chars.

[Peter Kasting, 2011/04/01 20:19:20]

Nit: These comments are more useful than the ones you currently have on these
constants' declarations in title_prefix_matcher.h (which mostly just restate the
constants' names).  Can you delete this comment and put something like it there?

[MAD, 2011/04/05 16:13:12]

Good idea..
Done.

+    // Also, we want to show the last few common chars, so we only do it if
+    // the common prefix is long enough, otherwise, we could be
+    // replacing less characters than the ellipsis take... not worth it.
+    if (data().common_prefix_length > TitlePrefixMatcher::kMinElidingLength &&
+        font_->GetExpectedTextWidth(6) < title_bounds.width() &&
         font_->GetStringWidth(title) > title_bounds.width()) {
-      title.replace(0, data().common_prefix_length, UTF8ToUTF16(ui::kEllipsis));
+      title.replace(0,
+                    data().common_prefix_length -
+                        TitlePrefixMatcher::kCommonCharsToShow,
+                    UTF8ToUTF16(ui::kEllipsis));
     }
   }
   canvas->DrawStringInt(title, *font_, title_color,
                         title_bounds.x(), title_bounds.y(),
                         title_bounds.width(), title_bounds.height());
 }
 
 void BaseTab::AnimationProgressed(const ui::Animation* animation) {
   SchedulePaint();
 }
@@ skipping 76 matching lines @@
 // static
 void BaseTab::InitResources() {
   static bool initialized = false;
   if (!initialized) {
     initialized = true;
     font_ = new gfx::Font(
         ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BaseFont));
     font_height_ = font_->GetHeight();
   }
 }