Commit 3b3914d7 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Remove FontList members of DownloadItemView.

Instead, use the affected labels themselves to do metric calculations,
or, where FontLists are necessary, to obtain the FontLists.

This makes a couple of behavioral changes:
(1) The "opening" string no longer assumes that the string length with
    the filename = the string length without the filename + the filename
    length.  This isn't a valid assumption in general (due to e.g.
    kerning).  Instead it just subs in the normally-elided filename.
    This isn't ideal; really we'd like to do filename eliding on the
    full string at the point where the filename subs in, but no APIs
    exist for that today.
(2) The filename is no longer abbreviated when showing the "prompt for
    scanning" dialog, since that's not inside the shelf and eliding is
    unnecessary (and maybe harmful).

This will also make it possible to style the filename inside
|file_name_label_| as is already done for other labels, but that change
isn't in this CL.

In theory, this would also behave better if the warning/deep scanning/
filename labels used different font sizes (but in practice they don't).

Bug: none
Change-Id: I5b6a0607a86b090204f65ff05151340b94d3a53d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315438
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791584}
parent b8ef479e
...@@ -70,6 +70,7 @@ ...@@ -70,6 +70,7 @@
#include "ui/gfx/animation/tween.h" #include "ui/gfx/animation/tween.h"
#include "ui/gfx/canvas.h" #include "ui/gfx/canvas.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
#include "ui/gfx/font_list.h"
#include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/insets.h"
#include "ui/gfx/geometry/point.h" #include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -254,10 +255,6 @@ DownloadItemView::DownloadItemView(DownloadUIModel::DownloadUIModelPtr download, ...@@ -254,10 +255,6 @@ DownloadItemView::DownloadItemView(DownloadUIModel::DownloadUIModelPtr download,
views::InstallRectHighlightPathGenerator(this); views::InstallRectHighlightPathGenerator(this);
model_->AddObserver(this); model_->AddObserver(this);
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
font_list_ = rb.GetFontListWithDelta(1);
status_font_list_ = rb.GetFontListWithDelta(-2);
// TODO(pkasting): Use bespoke file-scope subclasses for some of these child // TODO(pkasting): Use bespoke file-scope subclasses for some of these child
// views to localize functionality and simplify this class. // views to localize functionality and simplify this class.
...@@ -265,15 +262,12 @@ DownloadItemView::DownloadItemView(DownloadUIModel::DownloadUIModelPtr download, ...@@ -265,15 +262,12 @@ DownloadItemView::DownloadItemView(DownloadUIModel::DownloadUIModelPtr download,
open_button->set_context_menu_controller(this); open_button->set_context_menu_controller(this);
open_button_ = AddChildView(std::move(open_button)); open_button_ = AddChildView(std::move(open_button));
int file_name_style = views::style::STYLE_PRIMARY; auto file_name_label =
#if !defined(OS_LINUX) std::make_unique<views::StyledLabel>(base::string16(), nullptr);
if (UseNewWarnings()) file_name_label->SetTextContext(CONTEXT_DOWNLOAD_SHELF);
file_name_style = STYLE_EMPHASIZED;
#endif
auto file_name_label = std::make_unique<views::Label>(
ElidedFilename(), CONTEXT_DOWNLOAD_SHELF, file_name_style);
file_name_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
file_name_label->GetViewAccessibility().OverrideIsIgnored(true); file_name_label->GetViewAccessibility().OverrideIsIgnored(true);
const base::string16 filename = ElidedFilename(*file_name_label);
file_name_label->SetText(filename);
file_name_label_ = AddChildView(std::move(file_name_label)); file_name_label_ = AddChildView(std::move(file_name_label));
auto status_label = std::make_unique<views::Label>( auto status_label = std::make_unique<views::Label>(
...@@ -385,26 +379,26 @@ void DownloadItemView::Layout() { ...@@ -385,26 +379,26 @@ void DownloadItemView::Layout() {
gfx::Rect(child_origin, open_now_button_->GetPreferredSize())); gfx::Rect(child_origin, open_now_button_->GetPreferredSize()));
} }
} else { } else {
int mirrored_x = GetMirroredXWithWidthInView( const int mirrored_x = GetMirroredXWithWidthInView(
kStartPadding + kProgressIndicatorSize + kProgressTextPadding, kStartPadding + kProgressIndicatorSize + kProgressTextPadding,
kTextWidth); kTextWidth);
int text_height = font_list_.GetHeight(); int text_height = file_name_label_->GetLineHeight();
if (!status_label_->GetText().empty()) if (!status_label_->GetText().empty())
text_height += status_font_list_.GetHeight(); text_height += status_label_->GetLineHeight();
int file_name_y = CenterY(text_height); const int file_name_y = CenterY(text_height);
file_name_label_->SetBoundsRect( file_name_label_->SetBounds(mirrored_x, file_name_y, kTextWidth,
gfx::Rect(mirrored_x, file_name_y, kTextWidth, font_list_.GetHeight())); file_name_label_->GetPreferredSize().height());
int status_y = file_name_y + font_list_.GetHeight(); const int status_y = file_name_y + file_name_label_->GetLineHeight();
bool should_expand_for_status_text = const bool should_expand_for_status_text =
(model_->GetDangerType() == (model_->GetDangerType() ==
download::DOWNLOAD_DANGER_TYPE_DEEP_SCANNED_SAFE); download::DOWNLOAD_DANGER_TYPE_DEEP_SCANNED_SAFE);
int status_width = should_expand_for_status_text const gfx::Size status_size = status_label_->GetPreferredSize();
? status_label_->GetPreferredSize().width() const int status_width =
: kTextWidth; should_expand_for_status_text ? status_size.width() : kTextWidth;
status_label_->SetBoundsRect(gfx::Rect(mirrored_x, status_y, status_width, status_label_->SetBoundsRect(
status_font_list_.GetHeight())); gfx::Rect(mirrored_x, status_y, status_width, status_size.height()));
} }
if (mode_ != Mode::kDangerous) { if (mode_ != Mode::kDangerous) {
...@@ -545,26 +539,19 @@ void DownloadItemView::OnDownloadUpdated() { ...@@ -545,26 +539,19 @@ void DownloadItemView::OnDownloadUpdated() {
} }
void DownloadItemView::OnDownloadOpened() { void DownloadItemView::OnDownloadOpened() {
file_name_label_->SetTextStyle(views::style::STYLE_DISABLED); file_name_label_->SetDefaultTextStyle(views::style::STYLE_DISABLED);
// First, Calculate the download status opening string width. const base::string16 filename = ElidedFilename(*file_name_label_);
base::string16 status_string =
l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, base::string16());
int status_string_width = gfx::GetStringWidth(status_string, font_list_);
// Then, elide the file name.
base::string16 filename_string =
gfx::ElideFilename(model_->GetFileNameToReportUser(), font_list_,
kTextWidth - status_string_width);
// Last, concat the whole string to be set on the label.
file_name_label_->SetText( file_name_label_->SetText(
l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, filename_string)); l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, filename));
SetEnabled(false); SetEnabled(false);
const auto reenable = [](base::WeakPtr<DownloadItemView> view) { const auto reenable = [](base::WeakPtr<DownloadItemView> view) {
if (!view) if (!view)
return; return;
auto* label = view->file_name_label_; auto* label = view->file_name_label_;
label->SetTextStyle(views::style::STYLE_PRIMARY); label->SetDefaultTextStyle(views::style::STYLE_PRIMARY);
label->SetText(view->ElidedFilename()); const base::string16 filename = view->ElidedFilename(*label);
label->SetText(filename);
view->SetEnabled(true); view->SetEnabled(true);
}; };
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
...@@ -597,7 +584,8 @@ void DownloadItemView::MaybeSubmitDownloadToFeedbackService( ...@@ -597,7 +584,8 @@ void DownloadItemView::MaybeSubmitDownloadToFeedbackService(
gfx::Size DownloadItemView::CalculatePreferredSize() const { gfx::Size DownloadItemView::CalculatePreferredSize() const {
int width = 0; int width = 0;
// We set the height to the height of two rows or text plus margins. // We set the height to the height of two rows or text plus margins.
int child_height = font_list_.GetHeight() + status_font_list_.GetHeight(); int child_height =
file_name_label_->GetLineHeight() + status_label_->GetLineHeight();
if (has_warning_label(mode_)) { if (has_warning_label(mode_)) {
// Width. // Width.
...@@ -759,7 +747,7 @@ void DownloadItemView::OnThemeChanged() { ...@@ -759,7 +747,7 @@ void DownloadItemView::OnThemeChanged() {
SkColor background_color = SkColor background_color =
GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF); GetThemeProvider()->GetColor(ThemeProperties::COLOR_DOWNLOAD_SHELF);
file_name_label_->SetBackgroundColor(background_color); file_name_label_->SetDisplayedOnBackgroundColor(background_color);
status_label_->SetBackgroundColor(background_color); status_label_->SetBackgroundColor(background_color);
shelf_->ConfigureButtonForTheme(open_now_button_); shelf_->ConfigureButtonForTheme(open_now_button_);
...@@ -866,7 +854,7 @@ void DownloadItemView::UpdateLabels() { ...@@ -866,7 +854,7 @@ void DownloadItemView::UpdateLabels() {
warning_label_->SetVisible(has_warning_label(mode_)); warning_label_->SetVisible(has_warning_label(mode_));
if (warning_label_->GetVisible()) { if (warning_label_->GetVisible()) {
const base::string16 filename = ElidedFilename(); const base::string16 filename = ElidedFilename(*warning_label_);
size_t filename_offset; size_t filename_offset;
warning_label_->SetText(model_->GetWarningText(filename, &filename_offset)); warning_label_->SetText(model_->GetWarningText(filename, &filename_offset));
StyleFilename(*warning_label_, filename_offset, filename.length()); StyleFilename(*warning_label_, filename_offset, filename.length());
...@@ -880,7 +868,7 @@ void DownloadItemView::UpdateLabels() { ...@@ -880,7 +868,7 @@ void DownloadItemView::UpdateLabels() {
model_->download())) model_->download()))
? IDS_PROMPT_DEEP_SCANNING_DOWNLOAD ? IDS_PROMPT_DEEP_SCANNING_DOWNLOAD
: IDS_PROMPT_DEEP_SCANNING_APP_DOWNLOAD; : IDS_PROMPT_DEEP_SCANNING_APP_DOWNLOAD;
const base::string16 filename = ElidedFilename(); const base::string16 filename = ElidedFilename(*deep_scanning_label_);
size_t filename_offset; size_t filename_offset;
deep_scanning_label_->SetText( deep_scanning_label_->SetText(
l10n_util::GetStringFUTF16(id, filename, &filename_offset)); l10n_util::GetStringFUTF16(id, filename, &filename_offset));
...@@ -1171,8 +1159,11 @@ gfx::Size DownloadItemView::GetButtonSize() const { ...@@ -1171,8 +1159,11 @@ gfx::Size DownloadItemView::GetButtonSize() const {
return size; return size;
} }
base::string16 DownloadItemView::ElidedFilename() { base::string16 DownloadItemView::ElidedFilename(
return gfx::ElideFilename(model_->GetFileNameToReportUser(), font_list_, const views::StyledLabel& label) const {
const gfx::FontList& font_list =
views::style::GetFont(CONTEXT_DOWNLOAD_SHELF, GetFilenameStyle(label));
return gfx::ElideFilename(model_->GetFileNameToReportUser(), font_list,
kTextWidth); kTextWidth);
} }
...@@ -1230,7 +1221,7 @@ void DownloadItemView::ShowOpenDialog(content::WebContents* web_contents) { ...@@ -1230,7 +1221,7 @@ void DownloadItemView::ShowOpenDialog(content::WebContents* web_contents) {
web_contents); web_contents);
} else { } else {
safe_browsing::PromptForScanningModalDialog::ShowForWebContents( safe_browsing::PromptForScanningModalDialog::ShowForWebContents(
web_contents, ElidedFilename(), web_contents, model_->GetFileNameToReportUser().LossyDisplayName(),
base::BindOnce(&DownloadItemView::ExecuteCommand, base::BindOnce(&DownloadItemView::ExecuteCommand,
weak_ptr_factory_.GetWeakPtr(), weak_ptr_factory_.GetWeakPtr(),
DownloadCommands::DEEP_SCAN), DownloadCommands::DEEP_SCAN),
......
...@@ -23,7 +23,6 @@ ...@@ -23,7 +23,6 @@
#include "ui/base/resource/resource_bundle.h" #include "ui/base/resource/resource_bundle.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
#include "ui/gfx/animation/slide_animation.h" #include "ui/gfx/animation/slide_animation.h"
#include "ui/gfx/font_list.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h" #include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia.h"
...@@ -170,8 +169,8 @@ class DownloadItemView : public views::View, ...@@ -170,8 +169,8 @@ class DownloadItemView : public views::View,
gfx::Size GetButtonSize() const; gfx::Size GetButtonSize() const;
// Returns the file name to report to user. It might be elided to fit into // Returns the file name to report to user. It might be elided to fit into
// the text width. // the text width. |label| dictates the default text style.
base::string16 ElidedFilename(); base::string16 ElidedFilename(const views::StyledLabel& label) const;
// Returns the Y coordinate that centers |element_height| within the current // Returns the Y coordinate that centers |element_height| within the current
// height(). // height().
...@@ -218,12 +217,6 @@ class DownloadItemView : public views::View, ...@@ -218,12 +217,6 @@ class DownloadItemView : public views::View,
// The download shelf that owns us. // The download shelf that owns us.
DownloadShelfView* shelf_; DownloadShelfView* shelf_;
// The font list used to print the file name and warning text.
gfx::FontList font_list_;
// The font list used to print the status text below the file name.
gfx::FontList status_font_list_;
// Mode of the download item view. // Mode of the download item view.
Mode mode_; Mode mode_;
...@@ -250,7 +243,7 @@ class DownloadItemView : public views::View, ...@@ -250,7 +243,7 @@ class DownloadItemView : public views::View,
// used, so that we can detect a change in the path and reload the icon. // used, so that we can detect a change in the path and reload the icon.
base::FilePath file_path_; base::FilePath file_path_;
views::Label* file_name_label_; views::StyledLabel* file_name_label_;
views::Label* status_label_; views::Label* status_label_;
views::StyledLabel* warning_label_; views::StyledLabel* warning_label_;
views::StyledLabel* deep_scanning_label_; views::StyledLabel* deep_scanning_label_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment