From db9fff737c9ea46f704017bb2bb135904a09e330 Mon Sep 17 00:00:00 2001
From: "rsesek@chromium.org"
<rsesek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 22 Oct 2010 20:13:18 +0000
Subject: [Mac] Do not use the |title| field of the Page Info section, but the
optional |headline|.
* As a consequence of this, the view into which we draw |-isFlipped==YES| so
layout now happens right-side up.
* Do not show the cert button unless it would be enabled.
BUG=59611
TEST=No more headings for "Identity" or "Connection".
Review URL: http://codereview.chromium.org/3771010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63551 0039d316-1c4b-4281-b951-d872f2087c98
---
.../browser/cocoa/page_info_bubble_controller.mm | 139 ++++++++++++---------
.../cocoa/page_info_bubble_controller_unittest.mm | 35 ++++--
2 files changed, 104 insertions(+), 70 deletions(-)
(limited to 'chrome')
diff --git a/chrome/browser/cocoa/page_info_bubble_controller.mm b/chrome/browser/cocoa/page_info_bubble_controller.mm
index a2cc34f..b10c1cf 100644
--- a/chrome/browser/cocoa/page_info_bubble_controller.mm
+++ b/chrome/browser/cocoa/page_info_bubble_controller.mm
@@ -26,9 +26,9 @@
- (PageInfoModel*)model;
- (NSButton*)certificateButtonWithFrame:(NSRect)frame;
- (void)configureTextFieldAsLabel:(NSTextField*)textField;
-- (CGFloat)addTitleViewForInfo:(const PageInfoModel::SectionInfo&)info
- toSubviews:(NSMutableArray*)subviews
- atPoint:(NSPoint)point;
+- (CGFloat)addHeadlineViewForInfo:(const PageInfoModel::SectionInfo&)info
+ toSubviews:(NSMutableArray*)subviews
+ atPoint:(NSPoint)point;
- (CGFloat)addDescriptionViewForInfo:(const PageInfoModel::SectionInfo&)info
toSubviews:(NSMutableArray*)subviews
atPoint:(NSPoint)point;
@@ -45,6 +45,19 @@
parentWindow:(NSWindow*)parent;
@end
+// This simple NSView subclass is used as the single subview of the page info
+// bubble's window's contentView. Drawing is flipped so that layout of the
+// sections is easier. Apple recommends flipping the coordinate origin when
+// doing a lot of text layout because it's more natural.
+@interface PageInfoContentView : NSView {
+}
+@end
+@implementation PageInfoContentView
+- (BOOL)isFlipped {
+ return YES;
+}
+@end
+
namespace {
// The width of the window, in view coordinates. The height will be determined
@@ -57,8 +70,8 @@ const NSInteger kVerticalSpacing = 10;
// Padding along on the X-axis between the window frame and content.
const NSInteger kFramePadding = 20;
-// Spacing between the title and description text views.
-const NSInteger kTitleSpacing = 2;
+// Spacing between the optional headline and description text views.
+const NSInteger kHeadlineSpacing = 2;
// Spacing between the image and the text.
const NSInteger kImageSpacing = 10;
@@ -173,28 +186,33 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
// not using HTTPS.
- (void)performLayout {
// |offset| is the Y position that should be drawn at next.
- CGFloat offset = kVerticalSpacing;
+ CGFloat offset = kFramePadding;
// Keep the new subviews in an array that gets replaced at the end.
NSMutableArray* subviews = [NSMutableArray array];
- // First item, drawn at the bottom of the window, is the help center link.
- offset += [self addHelpButtonToSubviews:subviews atOffset:offset];
- offset += kVerticalSpacing;
- offset += [self addSeparatorToSubviews:subviews atOffset:offset];
-
- // Build the window from bottom-up because Cocoa's coordinate origin is the
- // lower left.
- for (int i = model_->GetSectionCount() - 1; i >= 0; --i) {
+ // The subviews will be attached to the PageInfoContentView, which has a
+ // flipped origin. This allows the code to build top-to-bottom.
+ const int sectionCount = model_->GetSectionCount();
+ for (int i = 0; i < sectionCount; ++i) {
PageInfoModel::SectionInfo info = model_->GetSectionInfo(i);
// Only certain sections have images. This affects the X position.
BOOL hasImage = model_->GetIconImage(info.icon_id) != nil;
CGFloat xPosition = (hasImage ? kTextXPosition : kTextXPositionNoImage);
- if (info.type == PageInfoModel::SECTION_INFO_IDENTITY) {
- offset += [self addCertificateButtonToSubviews:subviews
- atOffset:offset];
+ // Insert the image subview for sections that are appropriate.
+ CGFloat imageBaseline = offset + kImageSize;
+ if (hasImage) {
+ [self addImageViewForInfo:info toSubviews:subviews atOffset:offset];
+ }
+
+ // Add the title.
+ if (!info.headline.empty()) {
+ offset += [self addHeadlineViewForInfo:info
+ toSubviews:subviews
+ atPoint:NSMakePoint(xPosition, offset)];
+ offset += kHeadlineSpacing;
}
// Create the description of the state.
@@ -202,31 +220,37 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
toSubviews:subviews
atPoint:NSMakePoint(xPosition, offset)];
- // Add the title.
- offset += kTitleSpacing;
- offset += [self addTitleViewForInfo:info
- toSubviews:subviews
- atPoint:NSMakePoint(xPosition, offset)];
-
- // Insert the image subview for sections that are appropriate.
- if (hasImage) {
- [self addImageViewForInfo:info toSubviews:subviews atOffset:offset];
+ if (info.type == PageInfoModel::SECTION_INFO_IDENTITY && certID_) {
+ offset += kVerticalSpacing;
+ offset += [self addCertificateButtonToSubviews:subviews atOffset:offset];
}
+ // If at this point the description and optional headline and button are
+ // not as tall as the image, adjust the offset by the difference.
+ CGFloat imageBaselineDelta = imageBaseline - offset;
+ if (imageBaselineDelta > 0)
+ offset += imageBaselineDelta;
+
// Add the separators.
- if (i != 0) {
- offset += kVerticalSpacing;
- offset += [self addSeparatorToSubviews:subviews atOffset:offset];
- }
+ offset += kVerticalSpacing;
+ offset += [self addSeparatorToSubviews:subviews atOffset:offset];
}
- // Replace the window's content.
- [[[self window] contentView] setSubviews:subviews];
+ // The last item at the bottom of the window is the help center link.
+ offset += [self addHelpButtonToSubviews:subviews atOffset:offset];
+ offset += kVerticalSpacing;
- offset += kFramePadding;
+ // Create the dummy view that uses flipped coordinates.
+ NSRect contentFrame = NSMakeRect(0, 0, kWindowWidth, offset);
+ scoped_nsobject<PageInfoContentView> contentView(
+ [[PageInfoContentView alloc] initWithFrame:contentFrame]);
+ [contentView setSubviews:subviews];
- NSRect windowFrame = NSMakeRect(0, 0, kWindowWidth, 0);
- windowFrame.size.height += offset;
+ // Replace the window's content.
+ [[[self window] contentView] setSubviews:
+ [NSArray arrayWithObject:contentView]];
+
+ NSRect windowFrame = NSMakeRect(0, 0, kWindowWidth, offset);
windowFrame.size = [[[self window] contentView] convertSize:windowFrame.size
toView:nil];
// Adjust the origin by the difference in height.
@@ -273,21 +297,21 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
// Adds the title text field at the given x,y position, and returns the y
// position for the next element.
-- (CGFloat)addTitleViewForInfo:(const PageInfoModel::SectionInfo&)info
- toSubviews:(NSMutableArray*)subviews
- atPoint:(NSPoint)point {
+- (CGFloat)addHeadlineViewForInfo:(const PageInfoModel::SectionInfo&)info
+ toSubviews:(NSMutableArray*)subviews
+ atPoint:(NSPoint)point {
NSRect frame = NSMakeRect(point.x, point.y, kTextWidth, kImageSpacing);
- scoped_nsobject<NSTextField> titleField(
+ scoped_nsobject<NSTextField> textField(
[[NSTextField alloc] initWithFrame:frame]);
- [self configureTextFieldAsLabel:titleField.get()];
- [titleField setStringValue:base::SysUTF16ToNSString(info.title)];
+ [self configureTextFieldAsLabel:textField.get()];
+ [textField setStringValue:base::SysUTF16ToNSString(info.headline)];
NSFont* font = [NSFont boldSystemFontOfSize:[NSFont smallSystemFontSize]];
- [titleField setFont:font];
+ [textField setFont:font];
frame.size.height +=
[GTMUILocalizerAndLayoutTweaker sizeToFitFixedWidthTextField:
- titleField];
- [titleField setFrame:frame];
- [subviews addObject:titleField.get()];
+ textField];
+ [textField setFrame:frame];
+ [subviews addObject:textField.get()];
return NSHeight(frame);
}
@@ -315,6 +339,9 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
// Returns the y position for the next element.
- (CGFloat)addCertificateButtonToSubviews:(NSMutableArray*)subviews
atOffset:(CGFloat)offset {
+ // The certificate button should only be added if there is SSL information.
+ DCHECK(certID_);
+
// Create the certificate button. The frame will be fixed up by GTM, so
// use arbitrary values.
NSRect frame = NSMakeRect(kTextXPosition, offset, 100, 14);
@@ -323,19 +350,17 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
[GTMUILocalizerAndLayoutTweaker sizeToFitView:certButton];
// By default, assume that we don't have certificate information to show.
- [certButton setEnabled:NO];
- if (certID_) {
- scoped_refptr<net::X509Certificate> cert;
- CertStore::GetSharedInstance()->RetrieveCert(certID_, &cert);
-
- // Don't bother showing certificates if there isn't one. Gears runs
- // with no OS root certificate.
- if (cert.get() && cert->os_cert_handle()) {
- [certButton setEnabled:YES];
- }
+ scoped_refptr<net::X509Certificate> cert;
+ CertStore::GetSharedInstance()->RetrieveCert(certID_, &cert);
+
+ // Don't bother showing certificates if there isn't one. Gears runs
+ // with no OS root certificate.
+ if (!cert.get() || !cert->os_cert_handle()) {
+ // This should only ever happen in unit tests.
+ [certButton setEnabled:NO];
}
- return NSHeight(frame) + kVerticalSpacing;
+ return NSHeight([certButton frame]);
}
// Adds the state image at a pre-determined x position and the given y. This
@@ -344,7 +369,7 @@ void ShowPageInfoBubble(gfx::NativeWindow parent,
- (void)addImageViewForInfo:(const PageInfoModel::SectionInfo&)info
toSubviews:(NSMutableArray*)subviews
atOffset:(CGFloat)offset {
- NSRect frame = NSMakeRect(kFramePadding, offset - kImageSize, kImageSize,
+ NSRect frame = NSMakeRect(kFramePadding, offset, kImageSize,
kImageSize);
scoped_nsobject<NSImageView> imageView(
[[NSImageView alloc] initWithFrame:frame]);
diff --git a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
index eb7c177..da8a9db 100644
--- a/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
+++ b/chrome/browser/cocoa/page_info_bubble_controller_unittest.mm
@@ -22,11 +22,11 @@ class FakeModel : public PageInfoModel {
FakeModel() : PageInfoModel() {}
void AddSection(SectionStateIcon icon_id,
- const string16& title,
+ const string16& headline,
const string16& description,
SectionInfoType type) {
sections_.push_back(SectionInfo(
- icon_id, title, string16(), description, type));
+ icon_id, string16(), headline, description, type));
}
};
@@ -67,7 +67,13 @@ class PageInfoBubbleControllerTest : public CocoaTest {
int link_count = 1;
++spacer_count;
- for (NSView* view in [[window_ contentView] subviews]) {
+ // The window's only immediate child is an invisible view that has a flipped
+ // coordinate origin. It is into this that all views get placed.
+ NSArray* windowSubviews = [[window_ contentView] subviews];
+ EXPECT_EQ(1U, [windowSubviews count]);
+ NSArray* subviews = [[windowSubviews lastObject] subviews];
+
+ for (NSView* view in subviews) {
if ([view isKindOfClass:[NSTextField class]]) {
--text_count;
} else if ([view isKindOfClass:[NSImageView class]]) {
@@ -113,28 +119,28 @@ class PageInfoBubbleControllerTest : public CocoaTest {
TEST_F(PageInfoBubbleControllerTest, NoHistoryNoSecurity) {
model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+ string16(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY),
PageInfoModel::SECTION_INFO_IDENTITY);
model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+ string16(),
l10n_util::GetStringFUTF16(
IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
ASCIIToUTF16("google.com")),
PageInfoModel::SECTION_INFO_CONNECTION);
CreateBubble();
- CheckWindow(/*text=*/4, /*image=*/2, /*spacer=*/1, /*button=*/1);
+ CheckWindow(/*text=*/2, /*image=*/2, /*spacer=*/1, /*button=*/0);
}
TEST_F(PageInfoBubbleControllerTest, HistoryNoSecurity) {
model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+ string16(),
l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY),
PageInfoModel::SECTION_INFO_IDENTITY);
model_->AddSection(PageInfoModel::ICON_STATE_ERROR,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+ string16(),
l10n_util::GetStringFUTF16(
IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT,
ASCIIToUTF16("google.com")),
@@ -153,13 +159,13 @@ TEST_F(PageInfoBubbleControllerTest, HistoryNoSecurity) {
[controller_ performLayout];
- CheckWindow(/*text=*/6, /*image=*/3, /*spacer=*/2, /*button=*/1);
+ CheckWindow(/*text=*/4, /*image=*/3, /*spacer=*/2, /*button=*/0);
}
TEST_F(PageInfoBubbleControllerTest, NoHistoryMixedSecurity) {
model_->AddSection(PageInfoModel::ICON_STATE_OK,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_IDENTITY_TITLE),
+ string16(),
l10n_util::GetStringFUTF16(
IDS_PAGE_INFO_SECURITY_TAB_SECURE_IDENTITY,
ASCIIToUTF16("Goat Security Systems")),
@@ -177,17 +183,20 @@ TEST_F(PageInfoBubbleControllerTest, NoHistoryMixedSecurity) {
IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_INSECURE_CONTENT_WARNING));
model_->AddSection(PageInfoModel::ICON_STATE_OK,
- l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_CONNECTION_TITLE),
+ string16(),
description,
PageInfoModel::SECTION_INFO_CONNECTION);
+
CreateBubble();
+ [controller_ setCertID:1];
+ [controller_ performLayout];
- NSArray* subviews = [[window_ contentView] subviews];
- CheckWindow(/*text=*/4, /*image=*/2, /*spacer=*/1, /*button=*/1);
+ CheckWindow(/*text=*/2, /*image=*/2, /*spacer=*/1, /*button=*/1);
// Look for the over-sized box.
NSString* targetDesc = base::SysUTF16ToNSString(description);
+ NSArray* subviews = [[window_ contentView] subviews];
for (NSView* subview in subviews) {
if ([subview isKindOfClass:[NSTextField class]]) {
NSTextField* desc = static_cast<NSTextField*>(subview);
--
cgit v1.1