From d3586225e4a5249e4c862877dd5e606887cb6b9a Mon Sep 17 00:00:00 2001 From: treib Date: Mon, 15 Jun 2015 05:50:51 -0700 Subject: [PATCH] Clean up extension confirmation prompts and make them consistent between Views and Cocoa. All string changes + screenshots: https://docs.google.com/a/google.com/spreadsheets/d/1MVv3BjTtzNHTfUbsm4_fRBOSto7aE0E03IitDmvjg8k/edit?usp=sharing XIB changes in ExtensionInstallPromptWebstoreData.xib: Make the title NSTextField match the one in ExtensionInstallPrompt.xib (multi-line instead of eliding). BUG=419403 Review URL: https://codereview.chromium.org/1056003004 Cr-Commit-Position: refs/heads/master@{#334371} --- chrome/app/generated_resources.grd | 112 ++++++-------- .../nibs/ExtensionInstallPromptWebstoreData.xib | 4 +- .../api/identity_private/identity_private_api.cc | 2 +- chrome/browser/extensions/bundle_installer.cc | 6 +- chrome/browser/extensions/extension_disabled_ui.cc | 4 +- .../browser/extensions/extension_install_prompt.cc | 164 ++++++++------------- .../browser/extensions/extension_install_prompt.h | 6 +- .../browser/extensions/external_install_error.cc | 1 - .../extension_install_view_controller.mm | 24 ++- .../extensions/extension_install_dialog_view.cc | 103 ++++++------- .../extensions/extension_install_dialog_view.h | 13 +- 11 files changed, 190 insertions(+), 249 deletions(-) diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index e79a338e9ab1..d5833feb87d6 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -4239,61 +4239,25 @@ Even if you have downloaded files from this website before, the website might ha - - Confirm Installation - - - Confirm New App - - - Confirm New Extension - - - Confirm New Theme - Confirm Removal - - Confirm Re-enable - - + Confirm Permissions - - Current Permissions - - - Confirm Launch App - - - Repair Extension + + Add "$1Gmail Checker" for $2Wallace? - - Repair App - - - Confirm New App - - - Confirm New Extension - - - Confirm New App for $1Wallace - - - Confirm New Extension for $1Wallace - - + Add "$1Gmail Checker"? - + "$1My Awesome Bundle" adds these extensions: - + "$1My Awesome Bundle" adds these apps: - + "$1My Awesome Bundle" adds these apps and extensions: @@ -4302,22 +4266,22 @@ Even if you have downloaded files from this website before, the website might ha "$1Extension Manager" would like to remove "$2Gmail Checker". - + The newest version of "$1Gmail Checker" has been disabled because it requires more permissions. - + "$1Gmail Checker" has requested additional permissions. - - $1Gmail Checker + + Current Permissions for "$1Gmail Checker" - + Try "$1Gmail Checker"? - + Enable "$1Gmail Checker"? - + Repair "$1Gmail Checker"? @@ -5084,8 +5048,17 @@ Keep your key file in a safe place. You will need it to create new versions of y Add + + Add extension + + + Add app + + + Add theme + - Launch + Launch app Remove @@ -5103,19 +5076,25 @@ Keep your key file in a safe place. You will need it to create new versions of y Deny - Revoke File Access + Revoke file access - Revoke Device Access + Revoke device access - Revoke File and Device Access + Revoke file and device access - - Enable + + Enable extension + + + Enable app + + + Repair extension - - Repair + + Repair app Chrome Web Store @@ -5156,17 +5135,20 @@ Keep your key file in a safe place. You will need it to create new versions of y New theme added ($1Babylon Toolbar) - - "$1Babylon Toolbar" added + + Another program on your computer added an app that may change the way Chrome works. + +$1Babylon Toolbar - + Another program on your computer added an extension that may change the way Chrome works. + +$1Babylon Toolbar - - Another program on your computer added an app that may change the way Chrome works. - - + Another program on your computer added a theme that may change the way Chrome works. + +$1Babylon Toolbar Enable extension diff --git a/chrome/app/nibs/ExtensionInstallPromptWebstoreData.xib b/chrome/app/nibs/ExtensionInstallPromptWebstoreData.xib index 99264bcf7048..fd5f9f27f0e1 100644 --- a/chrome/app/nibs/ExtensionInstallPromptWebstoreData.xib +++ b/chrome/app/nibs/ExtensionInstallPromptWebstoreData.xib @@ -435,8 +435,8 @@ ARcABAAAAAEAAAACARwAAwAAAAEAAQAAAVIAAwAAAAEAAQAAAVMAAwAAAAIAAQABAAAAAA YES - 67108928 - 272631808 + 67108864 + 272629760 Title LucidaGrande diff --git a/chrome/browser/extensions/api/identity_private/identity_private_api.cc b/chrome/browser/extensions/api/identity_private/identity_private_api.cc index 47e2006b2e07..5c0e7f19767d 100644 --- a/chrome/browser/extensions/api/identity_private/identity_private_api.cc +++ b/chrome/browser/extensions/api/identity_private/identity_private_api.cc @@ -22,7 +22,7 @@ bool IdentityPrivateGetStringsFunction::RunSync() { dict->SetString( "window-title", - l10n_util::GetStringUTF16(IDS_EXTENSION_PERMISSIONS_PROMPT_TITLE)); + l10n_util::GetStringUTF16(IDS_EXTENSION_CONFIRM_PERMISSIONS)); return true; } diff --git a/chrome/browser/extensions/bundle_installer.cc b/chrome/browser/extensions/bundle_installer.cc index 133215ca4ddb..4c4af6ac63eb 100644 --- a/chrome/browser/extensions/bundle_installer.cc +++ b/chrome/browser/extensions/bundle_installer.cc @@ -66,9 +66,9 @@ scoped_refptr CreateDummyExtension( const int kHeadingIds[3][4] = { { 0, - IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_HEADING_EXTENSIONS, - IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_HEADING_APPS, - IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_HEADING_EXTENSION_APPS + IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_TITLE_EXTENSIONS, + IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_TITLE_APPS, + IDS_EXTENSION_BUNDLE_INSTALL_PROMPT_TITLE_EXTENSION_APPS }, { 0, diff --git a/chrome/browser/extensions/extension_disabled_ui.cc b/chrome/browser/extensions/extension_disabled_ui.cc index c5499b5d8863..79809fefe376 100644 --- a/chrome/browser/extensions/extension_disabled_ui.cc +++ b/chrome/browser/extensions/extension_disabled_ui.cc @@ -330,7 +330,9 @@ base::string16 ExtensionDisabledGlobalError::GetBubbleViewAcceptButtonLabel() { } if (is_remote_install_) { return l10n_util::GetStringUTF16( - IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON); + extension_->is_app() + ? IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_APP + : IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_EXTENSION); } return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_RE_ENABLE_BUTTON); } diff --git a/chrome/browser/extensions/extension_install_prompt.cc b/chrome/browser/extensions/extension_install_prompt.cc index cbc7a6e38a04..433d7da5de00 100644 --- a/chrome/browser/extensions/extension_install_prompt.cc +++ b/chrome/browser/extensions/extension_install_prompt.cc @@ -65,30 +65,17 @@ bool AllowWebstoreData(ExtensionInstallPrompt::PromptType type) { } static const int kTitleIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { - 0, // The regular install prompt depends on what's being installed. - IDS_EXTENSION_INLINE_INSTALL_PROMPT_TITLE, IDS_EXTENSION_INSTALL_PROMPT_TITLE, + IDS_EXTENSION_INSTALL_PROMPT_TITLE, + 0, // Heading for bundle installs depends on the bundle contents. IDS_EXTENSION_RE_ENABLE_PROMPT_TITLE, IDS_EXTENSION_PERMISSIONS_PROMPT_TITLE, - IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_TITLE, + 0, // External installs use different strings for extensions/apps/themes. IDS_EXTENSION_POST_INSTALL_PERMISSIONS_PROMPT_TITLE, IDS_EXTENSION_LAUNCH_APP_PROMPT_TITLE, - 0, // The remote install prompt depends on what's being installed. - 0, // The repair install prompt depends on what's being installed. - 0, // The delegated install prompt depends on what's being installed. -}; -static const int kHeadingIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { - IDS_EXTENSION_INSTALL_PROMPT_HEADING, - 0, // Inline installs use the extension name. - 0, // Heading for bundle installs depends on the bundle contents. - IDS_EXTENSION_RE_ENABLE_PROMPT_HEADING, - IDS_EXTENSION_PERMISSIONS_PROMPT_HEADING, - 0, // External installs use different strings for extensions/apps. - IDS_EXTENSION_POST_INSTALL_PERMISSIONS_PROMPT_HEADING, - IDS_EXTENSION_LAUNCH_APP_PROMPT_HEADING, - IDS_EXTENSION_REMOTE_INSTALL_PROMPT_HEADING, - IDS_EXTENSION_REPAIR_PROMPT_HEADING, - IDS_EXTENSION_INSTALL_PROMPT_HEADING, + IDS_EXTENSION_REMOTE_INSTALL_PROMPT_TITLE, + IDS_EXTENSION_REPAIR_PROMPT_TITLE, + IDS_EXTENSION_DELEGATED_INSTALL_PROMPT_TITLE, }; static const int kButtons[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, @@ -97,6 +84,9 @@ static const int kButtons[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, + // The "OK" button in the post install permissions dialog allows revoking + // file/device access, and is only shown if such permissions exist; see + // ShouldDisplayRevokeButton(). ui::DIALOG_BUTTON_CANCEL, ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, @@ -104,30 +94,30 @@ static const int kButtons[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL, }; static const int kAcceptButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { - IDS_EXTENSION_PROMPT_INSTALL_BUTTON, - IDS_EXTENSION_PROMPT_INSTALL_BUTTON, + 0, // Regular installs use different strings for extensions/apps/themes. + 0, // Inline installs as well. IDS_EXTENSION_PROMPT_INSTALL_BUTTON, IDS_EXTENSION_PROMPT_RE_ENABLE_BUTTON, IDS_EXTENSION_PROMPT_PERMISSIONS_BUTTON, - 0, // External installs use different strings for extensions/apps. + 0, // External installs use different strings for extensions/apps/themes. 0, // Different strings depending on the files and devices retained. IDS_EXTENSION_PROMPT_LAUNCH_BUTTON, - IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON, - IDS_EXTENSION_PROMPT_REPAIR_BUTTON, - IDS_EXTENSION_PROMPT_PERMISSIONS_BUTTON, + 0, // Remote installs use different strings for extensions/apps. + 0, // Repairs use different strings for extensions/apps. + 0, // Delegated installs use different strings for extensions/apps/themes. }; static const int kAbortButtonIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { - 0, // These all use the platform's default cancel label. - 0, - 0, - 0, + IDS_CANCEL, + IDS_CANCEL, + IDS_CANCEL, + IDS_CANCEL, IDS_EXTENSION_PROMPT_PERMISSIONS_ABORT_BUTTON, IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_ABORT_BUTTON, IDS_CLOSE, - 0, // Platform dependent cancel button. - 0, - 0, - IDS_EXTENSION_PROMPT_PERMISSIONS_ABORT_BUTTON, + IDS_CANCEL, + IDS_CANCEL, + IDS_CANCEL, + IDS_CANCEL, }; static const int kPermissionsHeaderIds[ExtensionInstallPrompt::NUM_PROMPT_TYPES] = { @@ -314,59 +304,23 @@ void ExtensionInstallPrompt::Prompt::SetWebstoreData( } base::string16 ExtensionInstallPrompt::Prompt::GetDialogTitle() const { - int resource_id = kTitleIds[type_]; - - if (type_ == INSTALL_PROMPT) { - if (extension_->is_app()) - resource_id = IDS_EXTENSION_INSTALL_APP_PROMPT_TITLE; - else if (extension_->is_theme()) - resource_id = IDS_EXTENSION_INSTALL_THEME_PROMPT_TITLE; - else - resource_id = IDS_EXTENSION_INSTALL_EXTENSION_PROMPT_TITLE; - } else if (type_ == EXTERNAL_INSTALL_PROMPT) { - return l10n_util::GetStringFUTF16( - resource_id, base::UTF8ToUTF16(extension_->name())); - } else if (type_ == REMOTE_INSTALL_PROMPT) { - if (extension_->is_app()) - resource_id = IDS_EXTENSION_REMOTE_INSTALL_APP_PROMPT_TITLE; - else - resource_id = IDS_EXTENSION_REMOTE_INSTALL_EXTENSION_PROMPT_TITLE; - } else if (type_ == REPAIR_PROMPT) { - if (extension_->is_app()) - resource_id = IDS_EXTENSION_REPAIR_APP_PROMPT_TITLE; - else - resource_id = IDS_EXTENSION_REPAIR_EXTENSION_PROMPT_TITLE; - } else if (type_ == DELEGATED_PERMISSIONS_PROMPT) { - DCHECK(!delegated_username_.empty()); - if (extension_->is_app()) - resource_id = IDS_EXTENSION_DELEGATED_INSTALL_APP_PROMPT_TITLE; - else - resource_id = IDS_EXTENSION_DELEGATED_INSTALL_EXTENSION_PROMPT_TITLE; - return l10n_util::GetStringFUTF16( - resource_id, base::UTF8ToUTF16(delegated_username_)); - } - - return l10n_util::GetStringUTF16(resource_id); -} - -base::string16 ExtensionInstallPrompt::Prompt::GetHeading() const { - if (type_ == INLINE_INSTALL_PROMPT) { - return base::UTF8ToUTF16(extension_->name()); - } else if (type_ == BUNDLE_INSTALL_PROMPT) { + int id = kTitleIds[type_]; + if (type_ == BUNDLE_INSTALL_PROMPT) { return bundle_->GetHeadingTextFor(BundleInstaller::Item::STATE_PENDING); - } else if (type_ == EXTERNAL_INSTALL_PROMPT) { - int resource_id = -1; + } + if (type_ == DELEGATED_PERMISSIONS_PROMPT) { + return l10n_util::GetStringFUTF16(id, base::UTF8ToUTF16(extension_->name()), + base::UTF8ToUTF16(delegated_username_)); + } + if (type_ == EXTERNAL_INSTALL_PROMPT) { if (extension_->is_app()) - resource_id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_HEADING_APP; + id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_TITLE_APP; else if (extension_->is_theme()) - resource_id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_HEADING_THEME; + id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_TITLE_THEME; else - resource_id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_HEADING_EXTENSION; - return l10n_util::GetStringUTF16(resource_id); - } else { - return l10n_util::GetStringFUTF16( - kHeadingIds[type_], base::UTF8ToUTF16(extension_->name())); + id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_TITLE_EXTENSION; } + return l10n_util::GetStringFUTF16(id, base::UTF8ToUTF16(extension_->name())); } int ExtensionInstallPrompt::Prompt::GetDialogButtons() const { @@ -377,48 +331,50 @@ int ExtensionInstallPrompt::Prompt::GetDialogButtons() const { return kButtons[type_]; } -bool ExtensionInstallPrompt::Prompt::HasAcceptButtonLabel() const { - if (type_ == POST_INSTALL_PERMISSIONS_PROMPT) - return ShouldDisplayRevokeButton(); - - if (kAcceptButtonIds[type_] == 0) - return false; - - return true; -} - base::string16 ExtensionInstallPrompt::Prompt::GetAcceptButtonLabel() const { - if (type_ == EXTERNAL_INSTALL_PROMPT) { - int id = -1; + int id = kAcceptButtonIds[type_]; + + if (type_ == INSTALL_PROMPT || type_ == INLINE_INSTALL_PROMPT || + type_ == DELEGATED_PERMISSIONS_PROMPT) { + if (extension_->is_app()) + id = IDS_EXTENSION_INSTALL_PROMPT_ACCEPT_BUTTON_APP; + else if (extension_->is_theme()) + id = IDS_EXTENSION_INSTALL_PROMPT_ACCEPT_BUTTON_THEME; + else + id = IDS_EXTENSION_INSTALL_PROMPT_ACCEPT_BUTTON_EXTENSION; + } else if (type_ == EXTERNAL_INSTALL_PROMPT) { if (extension_->is_app()) id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_ACCEPT_BUTTON_APP; else if (extension_->is_theme()) id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_ACCEPT_BUTTON_THEME; else id = IDS_EXTENSION_EXTERNAL_INSTALL_PROMPT_ACCEPT_BUTTON_EXTENSION; - return l10n_util::GetStringUTF16(id); } else if (type_ == POST_INSTALL_PERMISSIONS_PROMPT) { - int id = -1; if (GetRetainedFileCount() && GetRetainedDeviceCount()) { id = IDS_EXTENSION_PROMPT_PERMISSIONS_CLEAR_RETAINED_FILES_AND_DEVICES_BUTTON; } else if (GetRetainedFileCount()) { id = IDS_EXTENSION_PROMPT_PERMISSIONS_CLEAR_RETAINED_FILES_BUTTON; - } else { - DCHECK_LT(0U, GetRetainedDeviceCount()); + } else if (GetRetainedDeviceCount()) { id = IDS_EXTENSION_PROMPT_PERMISSIONS_CLEAR_RETAINED_DEVICES_BUTTON; } - return l10n_util::GetStringUTF16(id); + // If there are neither retained files nor devices, leave id 0 so there + // will be no "accept" button. + } else if (type_ == REMOTE_INSTALL_PROMPT) { + if (extension_->is_app()) + id = IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_APP; + else + id = IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_EXTENSION; + } else if (type_ == REPAIR_PROMPT) { + if (extension_->is_app()) + id = IDS_EXTENSION_PROMPT_REPAIR_BUTTON_APP; + else + id = IDS_EXTENSION_PROMPT_REPAIR_BUTTON_EXTENSION; } - return l10n_util::GetStringUTF16(kAcceptButtonIds[type_]); -} - -bool ExtensionInstallPrompt::Prompt::HasAbortButtonLabel() const { - return kAbortButtonIds[type_] > 0; + return id ? l10n_util::GetStringUTF16(id) : base::string16(); } base::string16 ExtensionInstallPrompt::Prompt::GetAbortButtonLabel() const { - CHECK(HasAbortButtonLabel()); return l10n_util::GetStringUTF16(kAbortButtonIds[type_]); } diff --git a/chrome/browser/extensions/extension_install_prompt.h b/chrome/browser/extensions/extension_install_prompt.h index 1613d2b355cb..32243c5aba08 100644 --- a/chrome/browser/extensions/extension_install_prompt.h +++ b/chrome/browser/extensions/extension_install_prompt.h @@ -116,11 +116,9 @@ class ExtensionInstallPrompt // Getters for UI element labels. base::string16 GetDialogTitle() const; - base::string16 GetHeading() const; int GetDialogButtons() const; - bool HasAcceptButtonLabel() const; + // Returns the empty string when there should be no "accept" button. base::string16 GetAcceptButtonLabel() const; - bool HasAbortButtonLabel() const; base::string16 GetAbortButtonLabel() const; base::string16 GetPermissionsHeading( PermissionsType permissions_type) const; @@ -130,7 +128,7 @@ class ExtensionInstallPrompt bool ShouldShowPermissions() const; // Getters for webstore metadata. Only populated when the type is - // INLINE_INSTALL_PROMPT. + // INLINE_INSTALL_PROMPT, EXTERNAL_INSTALL_PROMPT, or REPAIR_PROMPT. // The star display logic replicates the one used by the webstore (from // components.ratingutils.setFractionalYellowStars). Callers pass in an diff --git a/chrome/browser/extensions/external_install_error.cc b/chrome/browser/extensions/external_install_error.cc index 730b475c419e..306fa30a7a16 100644 --- a/chrome/browser/extensions/external_install_error.cc +++ b/chrome/browser/extensions/external_install_error.cc @@ -210,7 +210,6 @@ ExternalInstallBubbleAlert::GetBubbleViewMessages() { ExtensionInstallPrompt::PermissionsType::WITHHELD_PERMISSIONS; std::vector messages; - messages.push_back(prompt_->GetHeading()); if (prompt_->GetPermissionCount(regular_permissions)) { messages.push_back(prompt_->GetPermissionsHeading(regular_permissions)); for (size_t i = 0; i < prompt_->GetPermissionCount(regular_permissions); diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm index 75c72aef65c7..6654389b31e7 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm @@ -278,19 +278,19 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) { - (void)awakeFromNib { // Set control labels. - [titleField_ setStringValue:base::SysUTF16ToNSString(prompt_->GetHeading())]; + [titleField_ setStringValue:base::SysUTF16ToNSString( + prompt_->GetDialogTitle())]; NSRect okButtonRect; - if (prompt_->HasAcceptButtonLabel()) { - [okButton_ setTitle:base::SysUTF16ToNSString( - prompt_->GetAcceptButtonLabel())]; + base::string16 acceptButtonLabel = prompt_->GetAcceptButtonLabel(); + if (!acceptButtonLabel.empty()) { + [okButton_ setTitle:base::SysUTF16ToNSString(acceptButtonLabel)]; } else { [okButton_ removeFromSuperview]; okButtonRect = [okButton_ frame]; okButton_ = nil; } - [cancelButton_ setTitle:prompt_->HasAbortButtonLabel() ? - base::SysUTF16ToNSString(prompt_->GetAbortButtonLabel()) : - l10n_util::GetNSString(IDS_CANCEL)]; + [cancelButton_ setTitle:base::SysUTF16ToNSString( + prompt_->GetAbortButtonLabel())]; if ([self hasWebstoreData]) { prompt_->AppendRatingStars(AppendRatingStarsShim, self); [ratingCountField_ setStringValue:base::SysUTF16ToNSString( @@ -314,6 +314,16 @@ bool HasAttribute(id item, CellAttributesMask attributeMask) { OffsetControlVerticallyToFitContent(titleField_, &totalOffset); + if ([self hasWebstoreData]) { + OffsetControlVerticallyToFitContent(ratingCountField_, &totalOffset); + OffsetViewVerticallyToFitContent(ratingStars_, &totalOffset); + OffsetControlVerticallyToFitContent(userCountField_, &totalOffset); + OffsetControlVerticallyToFitContent(storeLinkButton_, &totalOffset); + NSPoint separatorOrigin = [warningsSeparator_ frame].origin; + separatorOrigin.y -= totalOffset; + [warningsSeparator_ setFrameOrigin:separatorOrigin]; + } + // Resize |okButton_| and |cancelButton_| to fit the button labels, but keep // them right-aligned. NSSize buttonDelta; diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc index 555c2906963b..d570d3365994 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc @@ -204,10 +204,10 @@ void ExtensionInstallDialogView::InitView() { // Inline install // w/ permissions no permissions // +--------------------+------+ +--------------+------+ - // | heading | icon | | heading | icon | + // | title | icon | | title | icon | // +--------------------| | +--------------| | // | rating | | | rating | | - // +--------------------| | +--------------+ | + // +--------------------| | +--------------| | // | user_count | | | user_count | | // +--------------------| | +--------------| | // | store_link | | | store_link | | @@ -216,21 +216,21 @@ void ExtensionInstallDialogView::InitView() { // +--------------------+------+ // | permissions_header | | // +--------------------+------+ - // | permission1 | | + // | * permission1 | | // +--------------------+------+ - // | permission2 | | + // | * permission2 | | // +--------------------+------+ // // Regular install // w/ permissions no permissions // +--------------------+------+ +--------------+------+ - // | heading | icon | | heading | icon | + // | title | icon | | title | icon | // +--------------------| | +--------------+------+ // | permissions_header | | // +--------------------| | - // | permission1 | | + // | * permission1 | | // +--------------------| | - // | permission2 | | + // | * permission2 | | // +--------------------+------+ int left_column_width = (prompt_->ShouldShowPermissions() + prompt_->GetRetainedFileCount()) > 0 @@ -247,8 +247,8 @@ void ExtensionInstallDialogView::InitView() { // Create the full scrollable view which will contain all the information // including the permissions. scrollable_ = new CustomScrollableView(); - views::GridLayout* layout = CreateLayout( - scrollable_, left_column_width, column_set_id, false); + views::GridLayout* layout = + CreateLayout(scrollable_, left_column_width, column_set_id); ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); scroll_view_->SetContents(scrollable_); @@ -429,7 +429,7 @@ bool ExtensionInstallDialogView::AddPermissions( return false; layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing); - if (is_inline_install()) { + if (prompt_->has_webstore_data()) { layout->StartRow(0, column_set_id); layout->AddView(new views::Separator(views::Separator::HORIZONTAL), 3, @@ -489,21 +489,22 @@ bool ExtensionInstallDialogView::AddPermissions( views::GridLayout* ExtensionInstallDialogView::CreateLayout( views::View* parent, int left_column_width, - int column_set_id, - bool single_detail_row) const { - views::GridLayout* layout = views::GridLayout::CreatePanel(parent); + int column_set_id) const { + // This is basically views::GridLayout::CreatePanel, but without a top margin + // (we effectively get a top margin anyway from the empty dialog title). + views::GridLayout* layout = new views::GridLayout(parent); + layout->SetInsets(0, views::kButtonHEdgeMarginNew, views::kPanelVertMargin, + views::kButtonHEdgeMarginNew); parent->SetLayoutManager(layout); views::ColumnSet* column_set = layout->AddColumnSet(column_set_id); - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::FILL, + column_set->AddColumn(views::GridLayout::LEADING, views::GridLayout::LEADING, 0, // no resizing views::GridLayout::USE_PREF, 0, // no fixed width left_column_width); column_set->AddPaddingColumn(0, views::kPanelHorizMargin); - column_set->AddColumn(views::GridLayout::TRAILING, - views::GridLayout::LEADING, + column_set->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING, 0, // no resizing views::GridLayout::USE_PREF, 0, // no fixed width @@ -511,14 +512,14 @@ views::GridLayout* ExtensionInstallDialogView::CreateLayout( layout->StartRow(0, column_set_id); - ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - - views::Label* heading = new views::Label( - prompt_->GetHeading(), rb.GetFontList(ui::ResourceBundle::MediumFont)); - heading->SetMultiLine(true); - heading->SetHorizontalAlignment(gfx::ALIGN_LEFT); - heading->SizeToFit(left_column_width); - layout->AddView(heading); + views::Label* title = + new views::Label(prompt_->GetDialogTitle(), + ui::ResourceBundle::GetSharedInstance().GetFontList( + ui::ResourceBundle::MediumFont)); + title->SetMultiLine(true); + title->SetHorizontalAlignment(gfx::ALIGN_LEFT); + title->SizeToFit(left_column_width); + layout->AddView(title); // Scale down to icon size, but allow smaller icons (don't scale up). const gfx::ImageSkia* image = prompt_->icon().ToImageSkia(); @@ -530,26 +531,32 @@ views::GridLayout* ExtensionInstallDialogView::CreateLayout( icon->SetImage(*image); icon->SetHorizontalAlignment(views::ImageView::CENTER); icon->SetVerticalAlignment(views::ImageView::CENTER); - if (single_detail_row) { - layout->AddView(icon); - } else { - int icon_row_span = 1; - if (is_inline_install()) { - // Also span the rating, user_count and store_link rows. - icon_row_span = 4; - } else if (prompt_->ShouldShowPermissions()) { - size_t permission_count = prompt_->GetPermissionCount( - ExtensionInstallPrompt::PermissionsType::ALL_PERMISSIONS); - // Also span the permission header and each of the permission rows (all - // have a padding row above it). This also works for the 'no special - // permissions' case. - icon_row_span = 3 + permission_count * 2; - } else if (prompt_->GetRetainedFileCount()) { - // Also span the permission header and the retained files container. - icon_row_span = 4; - } - layout->AddView(icon, 1, icon_row_span); + + int icon_row_span = 1; + if (prompt_->has_webstore_data()) { + // Also span the rating, user_count and store_link rows. + icon_row_span = 4; + // Note: Do not span the permissions here, there's a separator in between! + } else if (prompt_->ShouldShowPermissions()) { + size_t permission_count = prompt_->GetPermissionCount( + ExtensionInstallPrompt::PermissionsType::ALL_PERMISSIONS); + // Also span the permission header and each of the permission rows (all + // have a padding row above it). This also works for the 'no special + // permissions' case. + icon_row_span = 3 + permission_count * 2; + } else if (prompt_->GetRetainedFileCount() || + prompt_->GetRetainedDeviceCount()) { + // Also span the permission header and the retained files container. + icon_row_span = 4; + } + if (is_bundle_install()) { + // Also span the list of bundle items. One row per item, plus a padding row + // above and below. + int bundle_item_count = prompt_->bundle()->CountItemsWithState( + BundleInstaller::Item::STATE_PENDING); + icon_row_span += bundle_item_count + 2; } + layout->AddView(icon, 1, icon_row_span); return layout; } @@ -572,9 +579,7 @@ base::string16 ExtensionInstallDialogView::GetDialogButtonLabel( case ui::DIALOG_BUTTON_OK: return prompt_->GetAcceptButtonLabel(); case ui::DIALOG_BUTTON_CANCEL: - return prompt_->HasAbortButtonLabel() - ? prompt_->GetAbortButtonLabel() - : l10n_util::GetStringUTF16(IDS_CANCEL); + return prompt_->GetAbortButtonLabel(); default: NOTREACHED(); return base::string16(); @@ -612,10 +617,6 @@ ui::ModalType ExtensionInstallDialogView::GetModalType() const { return ui::MODAL_TYPE_WINDOW; } -base::string16 ExtensionInstallDialogView::GetWindowTitle() const { - return prompt_->GetDialogTitle(); -} - void ExtensionInstallDialogView::LinkClicked(views::Link* source, int event_flags) { GURL store_url(extension_urls::GetWebstoreItemDetailURLPrefix() + diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view.h b/chrome/browser/ui/views/extensions/extension_install_dialog_view.h index 4d97b5048c5a..5729635536d7 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.h +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.h @@ -74,7 +74,6 @@ class ExtensionInstallDialogView : public views::DialogDelegateView, bool Cancel() override; bool Accept() override; ui::ModalType GetModalType() const override; - base::string16 GetWindowTitle() const override; void Layout() override; gfx::Size GetPreferredSize() const override; @@ -92,15 +91,9 @@ class ExtensionInstallDialogView : public views::DialogDelegateView, ExtensionInstallPrompt::PermissionsType perm_type); // Creates a layout consisting of dialog header, extension name and icon. - views::GridLayout* CreateLayout( - views::View* parent, - int left_column_width, - int column_set_id, - bool single_detail_row) const; - - bool is_inline_install() const { - return prompt_->type() == ExtensionInstallPrompt::INLINE_INSTALL_PROMPT; - } + views::GridLayout* CreateLayout(views::View* parent, + int left_column_width, + int column_set_id) const; bool is_bundle_install() const { return prompt_->type() == ExtensionInstallPrompt::BUNDLE_INSTALL_PROMPT; -- 2.11.4.GIT