Commit ef2654e4 authored by sail@chromium.org's avatar sail@chromium.org

Cocoa: Show OAuth issues in extension install dialog

This is the Cocoa version of the following two CLs:
https://chromiumcodereview.appspot.com/10696042
https://chromiumcodereview.appspot.com/10824054

Currently the extension install dialog shows permission warnings inside a multiline text field.

OAuth issues are different in that they are a list of issues, each of which can be expadned to show more details.

To support this I replaced the multiline text field with an NSOutlineView.

Screenshots:
  - OAuth issues + Warnings: http://i.imgur.com/Hbag3.png
  - Inline install + Warnings: http://i.imgur.com/gpLI9.png
  - Inline + No Warnings (outline view hidden): http://i.imgur.com/se9tH.png
  - Bundle install + Warnings: http://i.imgur.com/5H3Rm.png

XIB changes: Replaced warning and subtitle text field with outline view. Updated relevant outlets.

BUG=30206
TEST=
  1. Test OAuth issues. Run chrome with --demand-user-scope-approval. Drag/drop oauth2.crx from estade. Verify that warnings and oauth issues are displayed. Verify that expanding a oauth issue grows the dialog.
  2. Test Inline install + Warning. Navigate to https://www.google.com/chrome/intl/en/p/google-plus.html and click install. Verify that warnings are displayed.
  3. Test Inline Install + No Warnings. Navigate to http://chrome.angrybirds.com/ and click install. Verify that the outline view is hidden and that the dialog is correctly resized.
  4. Test Bundle Install. Run chrome with  --apps-gallery-url="http://www.corp.google.com/" Navigate to http://www.corp.google.com/~jstritar/bundle.html. Verify that warnings are shown.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151187 0039d316-1c4b-4281-b951-d872f2087c98
parent 733a58fc
This diff is collapsed.
...@@ -215,8 +215,7 @@ size_t ExtensionInstallPrompt::Prompt::GetPermissionCount() const { ...@@ -215,8 +215,7 @@ size_t ExtensionInstallPrompt::Prompt::GetPermissionCount() const {
string16 ExtensionInstallPrompt::Prompt::GetPermission(size_t index) const { string16 ExtensionInstallPrompt::Prompt::GetPermission(size_t index) const {
CHECK_LT(index, permissions_.size()); CHECK_LT(index, permissions_.size());
return l10n_util::GetStringFUTF16( return permissions_[index];
IDS_EXTENSION_PERMISSION_LINE, permissions_[index]);
} }
size_t ExtensionInstallPrompt::Prompt::GetOAuthIssueCount() const { size_t ExtensionInstallPrompt::Prompt::GetOAuthIssueCount() const {
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#import <Cocoa/Cocoa.h> #import <Cocoa/Cocoa.h>
#include "base/memory/scoped_nsobject.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/string16.h" #include "base/string16.h"
#include "chrome/browser/extensions/extension_install_prompt.h" #include "chrome/browser/extensions/extension_install_prompt.h"
...@@ -20,7 +21,9 @@ class PageNavigator; ...@@ -20,7 +21,9 @@ class PageNavigator;
// Displays the extension or bundle install prompt, and notifies the // Displays the extension or bundle install prompt, and notifies the
// ExtensionInstallPrompt::Delegate of success or failure. // ExtensionInstallPrompt::Delegate of success or failure.
@interface ExtensionInstallDialogController : NSWindowController { @interface ExtensionInstallDialogController : NSWindowController
<NSOutlineViewDataSource,
NSOutlineViewDelegate> {
@private @private
IBOutlet NSImageView* iconView_; IBOutlet NSImageView* iconView_;
IBOutlet NSTextField* titleField_; IBOutlet NSTextField* titleField_;
...@@ -28,9 +31,9 @@ class PageNavigator; ...@@ -28,9 +31,9 @@ class PageNavigator;
IBOutlet NSButton* cancelButton_; IBOutlet NSButton* cancelButton_;
IBOutlet NSButton* okButton_; IBOutlet NSButton* okButton_;
// Present only when the dialog has permission warnings to display. // Present only when the dialog has permission warnings or OAuth issues to
IBOutlet NSTextField* subtitleField_; // display.
IBOutlet NSTextField* warningsField_; IBOutlet NSOutlineView* outlineView_;
// Present only in the inline install dialog. // Present only in the inline install dialog.
IBOutlet NSBox* warningsSeparator_; // Only when there are permissions. IBOutlet NSBox* warningsSeparator_; // Only when there are permissions.
...@@ -42,16 +45,18 @@ class PageNavigator; ...@@ -42,16 +45,18 @@ class PageNavigator;
content::PageNavigator* navigator_; // weak content::PageNavigator* navigator_; // weak
ExtensionInstallPrompt::Delegate* delegate_; // weak ExtensionInstallPrompt::Delegate* delegate_; // weak
scoped_ptr<ExtensionInstallPrompt::Prompt> prompt_; scoped_ptr<ExtensionInstallPrompt::Prompt> prompt_;
scoped_nsobject<NSArray> warnings_;
BOOL isComputingRowHeight_;
} }
// For unit test use only // For unit test use only
@property(nonatomic, readonly) NSImageView* iconView; @property(nonatomic, readonly) NSImageView* iconView;
@property(nonatomic, readonly) NSTextField* titleField; @property(nonatomic, readonly) NSTextField* titleField;
@property(nonatomic, readonly) NSTextField* itemsField; @property(nonatomic, readonly) NSTextField* itemsField;
@property(nonatomic, readonly) NSTextField* subtitleField;
@property(nonatomic, readonly) NSTextField* warningsField;
@property(nonatomic, readonly) NSButton* cancelButton; @property(nonatomic, readonly) NSButton* cancelButton;
@property(nonatomic, readonly) NSButton* okButton; @property(nonatomic, readonly) NSButton* okButton;
@property(nonatomic, readonly) NSOutlineView* outlineView;
@property(nonatomic, readonly) NSBox* warningsSeparator; @property(nonatomic, readonly) NSBox* warningsSeparator;
@property(nonatomic, readonly) NSView* ratingStars; @property(nonatomic, readonly) NSView* ratingStars;
@property(nonatomic, readonly) NSTextField* ratingCountField; @property(nonatomic, readonly) NSTextField* ratingCountField;
......
...@@ -138,12 +138,12 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsNormalCancel) { ...@@ -138,12 +138,12 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsNormalCancel) {
EXPECT_TRUE([controller titleField] != nil); EXPECT_TRUE([controller titleField] != nil);
EXPECT_NE(0u, [[[controller titleField] stringValue] length]); EXPECT_NE(0u, [[[controller titleField] stringValue] length]);
EXPECT_TRUE([controller subtitleField] != nil); NSOutlineView* outlineView = [controller outlineView];
EXPECT_NE(0u, [[[controller subtitleField] stringValue] length]); EXPECT_TRUE(outlineView != nil);
EXPECT_NE('^', [[[controller subtitleField] stringValue] characterAtIndex:0]); EXPECT_EQ(2, [outlineView numberOfRows]);
EXPECT_NSEQ([[outlineView dataSource] outlineView:outlineView
EXPECT_TRUE([controller warningsField] != nil); objectValueForTableColumn:nil
EXPECT_NSEQ([[controller warningsField] stringValue], byItem:[outlineView itemAtRow:1]],
base::SysUTF16ToNSString(prompt.GetPermission(0))); base::SysUTF16ToNSString(prompt.GetPermission(0)));
EXPECT_TRUE([controller cancelButton] != nil); EXPECT_TRUE([controller cancelButton] != nil);
...@@ -231,11 +231,8 @@ TEST_F(ExtensionInstallDialogControllerTest, MultipleWarnings) { ...@@ -231,11 +231,8 @@ TEST_F(ExtensionInstallDialogControllerTest, MultipleWarnings) {
ASSERT_LT([[controller1 window] frame].size.height, ASSERT_LT([[controller1 window] frame].size.height,
[[controller2 window] frame].size.height); [[controller2 window] frame].size.height);
ASSERT_LT([[controller1 warningsField] frame].size.height, ASSERT_LT([[controller1 outlineView] frame].size.height,
[[controller2 warningsField] frame].size.height); [[controller2 outlineView] frame].size.height);
ASSERT_LT([[controller1 subtitleField] frame].origin.y,
[[controller2 subtitleField] frame].origin.y);
ASSERT_LT([[controller1 titleField] frame].origin.y, ASSERT_LT([[controller1 titleField] frame].origin.y,
[[controller2 titleField] frame].origin.y); [[controller2 titleField] frame].origin.y);
...@@ -281,8 +278,7 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsSkinny) { ...@@ -281,8 +278,7 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsSkinny) {
EXPECT_NE(0u, [[[controller okButton] stringValue] length]); EXPECT_NE(0u, [[[controller okButton] stringValue] length]);
EXPECT_NE('^', [[[controller okButton] stringValue] characterAtIndex:0]); EXPECT_NE('^', [[[controller okButton] stringValue] characterAtIndex:0]);
EXPECT_TRUE([controller subtitleField] == nil); EXPECT_TRUE([controller outlineView] == nil);
EXPECT_TRUE([controller warningsField] == nil);
} }
...@@ -336,10 +332,42 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsInline) { ...@@ -336,10 +332,42 @@ TEST_F(ExtensionInstallDialogControllerTest, BasicsInline) {
// Though we have no permissions warnings, these should still be hooked up, // Though we have no permissions warnings, these should still be hooked up,
// just invisible. // just invisible.
EXPECT_TRUE([controller subtitleField] != nil); EXPECT_TRUE([controller outlineView] != nil);
EXPECT_TRUE([[controller subtitleField] isHidden]); EXPECT_TRUE([[[controller outlineView] enclosingScrollView] isHidden]);
EXPECT_TRUE([controller warningsField] != nil);
EXPECT_TRUE([[controller warningsField] isHidden]);
EXPECT_TRUE([controller warningsSeparator] != nil); EXPECT_TRUE([controller warningsSeparator] != nil);
EXPECT_TRUE([[controller warningsSeparator] isHidden]); EXPECT_TRUE([[controller warningsSeparator] isHidden]);
} }
TEST_F(ExtensionInstallDialogControllerTest, OAuthIssues) {
MockExtensionInstallPromptDelegate delegate;
ExtensionInstallPrompt::Prompt prompt(
ExtensionInstallPrompt::INSTALL_PROMPT);
std::vector<string16> permissions;
permissions.push_back(UTF8ToUTF16("warning 1"));
prompt.SetPermissions(permissions);
IssueAdviceInfoEntry issue;
issue.description = UTF8ToUTF16("issue description 1");
issue.details.push_back(UTF8ToUTF16("issue detail 1"));
IssueAdviceInfo issues;
issues.push_back(issue);
prompt.SetOAuthIssueAdvice(issues);
prompt.set_extension(extension_.get());
prompt.set_icon(icon_);
scoped_nsobject<ExtensionInstallDialogController>
controller([[ExtensionInstallDialogController alloc]
initWithParentWindow:test_window()
navigator:browser()
delegate:&delegate
prompt:prompt]);
[controller window]; // force nib load
NSOutlineView* outlineView = [controller outlineView];
EXPECT_TRUE(outlineView != nil);
EXPECT_EQ(4, [outlineView numberOfRows]);
EXPECT_NSEQ([[outlineView dataSource] outlineView:outlineView
objectValueForTableColumn:nil
byItem:[outlineView itemAtRow:3]],
base::SysUTF16ToNSString(prompt.GetOAuthIssue(0).description));
}
...@@ -257,8 +257,9 @@ ExtensionInstallDialog::ExtensionInstallDialog( ...@@ -257,8 +257,9 @@ ExtensionInstallDialog::ExtensionInstallDialog(
FALSE, FALSE, 0); FALSE, FALSE, 0);
for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) { for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) {
GtkWidget* permission_label = gtk_label_new(UTF16ToUTF8( std::string permission = l10n_util::GetStringFUTF8(
prompt.GetPermission(i)).c_str()); IDS_EXTENSION_PERMISSION_LINE, prompt.GetPermission(i));
GtkWidget* permission_label = gtk_label_new(permission.c_str());
gtk_util::SetLabelWidth(permission_label, kLeftColumnMinWidth); gtk_util::SetLabelWidth(permission_label, kLeftColumnMinWidth);
gtk_box_pack_start(GTK_BOX(permissions_container), permission_label, gtk_box_pack_start(GTK_BOX(permissions_container), permission_label,
FALSE, FALSE, kPermissionsPadding); FALSE, FALSE, kPermissionsPadding);
......
...@@ -376,8 +376,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView( ...@@ -376,8 +376,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) { for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) {
layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing); layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
layout->StartRow(0, column_set_id); layout->StartRow(0, column_set_id);
views::Label* permission_label = new views::Label( views::Label* permission_label = new views::Label(PrepareForDisplay(
prompt.GetPermission(i)); prompt.GetPermission(i), true));
permission_label->SetMultiLine(true); permission_label->SetMultiLine(true);
permission_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT); permission_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
permission_label->SizeToFit(left_column_width); permission_label->SizeToFit(left_column_width);
......
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