From 0881617451f471982a6c86f25259656f992ccb12 Mon Sep 17 00:00:00 2001 From: Monica Dinculescu Date: Tue, 2 Sep 2014 15:15:39 -0400 Subject: [PATCH] [Merge] [Mac] Add tab and keyboard navigation to the new avatar bubble Tabbing should navigate through all the visible controls. I've changed the EditablePhotoButton to actually be a button and not just an image, so that tabbing and pressing enter actually works. BUG=405056 TBR=noms@chromium.org Review URL: https://codereview.chromium.org/489143002 Cr-Commit-Position: refs/heads/master@{#292455} (cherry picked from commit aaacc3030b8b46e4d0463d6d51f24341bf51ddfa) Review URL: https://codereview.chromium.org/533833003 Cr-Commit-Position: refs/branch-heads/2125@{#183} Cr-Branched-From: b68026d94bda36dd106a3d91a098719f952a9477-refs/heads/master@{#290040} --- .../profiles/profile_chooser_controller.mm | 172 ++++++++++-------- .../profile_chooser_controller_unittest.mm | 86 ++++++++- 2 files changed, 174 insertions(+), 84 deletions(-) diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm index 7198d96d9d1d6..4c78a0c961666 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm @@ -84,6 +84,7 @@ const int kBezelThickness = 3; // Width of the bezel on an NSButton. const int kImageTitleSpacing = 10; const int kBlueButtonHeight = 30; +const CGFloat kFocusRingLineWidth = 2; // Fixed size for embedded sign in pages as defined in Gaia. const CGFloat kFixedGaiaViewWidth = 360; @@ -409,22 +410,34 @@ - (NSSize)cellSize { return buttonSize; } -@end +- (NSFocusRingType)focusRingType { + // This is taken care of by the custom drawing code. + return NSFocusRingTypeNone; +} -// A custom button that has a transparent backround. -@interface TransparentBackgroundButton : NSButton -@end +- (void)drawWithFrame:(NSRect)frame inView:(NSView *)controlView { + [super drawInteriorWithFrame:frame inView:controlView]; -@implementation TransparentBackgroundButton -- (id)initWithFrame:(NSRect)frameRect { - if ((self = [super initWithFrame:frameRect])) { - [self setBordered:NO]; - [self setFont:[NSFont labelFontOfSize:kTextFontSize]]; - [self setButtonType:NSMomentaryChangeButton]; + // Focus ring. + if ([self showsFirstResponder]) { + NSRect focusRingRect = + NSInsetRect(frame, kFocusRingLineWidth, kFocusRingLineWidth); + // TODO(noms): When we are targetting 10.7, we should change this to use + // -drawFocusRingMaskWithFrame instead. + [[[NSColor keyboardFocusIndicatorColor] colorWithAlphaComponent:1] set]; + NSBezierPath* path = [NSBezierPath bezierPathWithRect:focusRingRect]; + [path setLineWidth:kFocusRingLineWidth]; + [path stroke]; } - return self; } +@end + +// A custom image view that has a transparent backround. +@interface TransparentBackgroundImageView : NSImageView +@end + +@implementation TransparentBackgroundImageView - (void)drawRect:(NSRect)dirtyRect { NSColor* backgroundColor = [NSColor colorWithCalibratedWhite:1 alpha:0.6f]; [backgroundColor setFill]; @@ -433,13 +446,31 @@ - (void)drawRect:(NSRect)dirtyRect { } @end +@interface CustomCircleImageCell : NSButtonCell +@end + +@implementation CustomCircleImageCell +- (void)drawWithFrame:(NSRect)frame inView:(NSView *)controlView { + // Display everything as a circle that spans the entire control. + NSBezierPath* path = [NSBezierPath bezierPathWithOvalInRect:frame]; + [path addClip]; + + [super drawImage:[self image] withFrame:frame inView:controlView]; + + // Focus ring. + if ([self showsFirstResponder]) { + [[[NSColor keyboardFocusIndicatorColor] colorWithAlphaComponent:1] set]; + [path setLineWidth:kFocusRingLineWidth]; + [path stroke]; + } +} +@end + // A custom image control that shows a "Change" button when moused over. -@interface EditableProfilePhoto : NSImageView { +@interface EditableProfilePhoto : HoverImageButton { @private AvatarMenu* avatarMenu_; // Weak; Owned by ProfileChooserController. - base::scoped_nsobject changePhotoButton_; - // Used to display the "Change" button on hover. - ui::ScopedCrTrackingArea trackingArea_; + base::scoped_nsobject changePhotoImage_; ProfileChooserController* controller_; } @@ -452,16 +483,6 @@ - (id)initWithFrame:(NSRect)frameRect // Called when the "Change" button is clicked. - (void)editPhoto:(id)sender; -// When hovering over the profile photo, show the "Change" button. -- (void)mouseEntered:(NSEvent*)event; - -// When hovering away from the profile photo, hide the "Change" button. -- (void)mouseExited:(NSEvent*)event; -@end - -@interface EditableProfilePhoto (Private) -// Create the "Change" avatar photo button. -- (TransparentBackgroundButton*)changePhotoButtonWithRect:(NSRect)rect; @end @implementation EditableProfilePhoto @@ -473,24 +494,29 @@ - (id)initWithFrame:(NSRect)frameRect if ((self = [super initWithFrame:frameRect])) { avatarMenu_ = avatarMenu; controller_ = controller; - [self setImage:CreateProfileImage( - profileIcon, kLargeImageSide).ToNSImage()]; - // Add a tracking area so that we can show/hide the button when hovering. - trackingArea_.reset([[CrTrackingArea alloc] - initWithRect:[self bounds] - options:NSTrackingMouseEnteredAndExited | NSTrackingActiveAlways - owner:self - userInfo:nil]); - [self addTrackingArea:trackingArea_.get()]; + [self setBordered:NO]; + + base::scoped_nsobject cell( + [[CustomCircleImageCell alloc] init]); + [self setCell:cell.get()]; + + [self setDefaultImage:CreateProfileImage( + profileIcon, kLargeImageSide).ToNSImage()]; + [self setImagePosition:NSImageOnly]; NSRect bounds = NSMakeRect(0, 0, kLargeImageSide, kLargeImageSide); if (editingAllowed) { - changePhotoButton_.reset([self changePhotoButtonWithRect:bounds]); - [self addSubview:changePhotoButton_]; - - // Hide the button until the image is hovered over. - [changePhotoButton_ setHidden:YES]; + [self setTarget:self]; + [self setAction:@selector(editPhoto:)]; + changePhotoImage_.reset([[TransparentBackgroundImageView alloc] + initWithFrame:bounds]); + [changePhotoImage_ setImage:ui::ResourceBundle::GetSharedInstance(). + GetNativeImageNamed(IDR_ICON_PROFILES_EDIT_CAMERA).AsNSImage()]; + [self addSubview:changePhotoImage_]; + + // Hide the image until the button is hovered over. + [changePhotoImage_ setHidden:YES]; } // Set the image cell's accessibility strings to be the same as the @@ -522,36 +548,15 @@ - (id)initWithFrame:(NSRect)frameRect return self; } -- (void)drawRect:(NSRect)dirtyRect { - NSRect bounds = [self bounds]; - - // Display the profile picture as a circle. - NSBezierPath* path = [NSBezierPath bezierPathWithOvalInRect:bounds]; - [path addClip]; - [self.image drawAtPoint:bounds.origin - fromRect:bounds - operation:NSCompositeSourceOver - fraction:1.0]; - -} - - (void)editPhoto:(id)sender { avatarMenu_->EditProfile(avatarMenu_->GetActiveProfileIndex()); [controller_ postActionPerformed:ProfileMetrics::PROFILE_DESKTOP_MENU_EDIT_IMAGE]; } -- (void)mouseEntered:(NSEvent*)event { - [changePhotoButton_ setHidden:NO]; -} - -- (void)mouseExited:(NSEvent*)event { - [changePhotoButton_ setHidden:YES]; -} - -// Make sure the element is focusable for accessibility. -- (BOOL)canBecomeKeyView { - return YES; +- (void)setHoverState:(HoverState)state { + [super setHoverState:state]; + [changePhotoImage_ setHidden:([self hoverState] == kHoverStateNone)]; } - (BOOL)accessibilityIsIgnored { @@ -571,16 +576,6 @@ - (void)accessibilityPerformAction:(NSString*)action { [super accessibilityPerformAction:action]; } -- (TransparentBackgroundButton*)changePhotoButtonWithRect:(NSRect)rect { - TransparentBackgroundButton* button = - [[TransparentBackgroundButton alloc] initWithFrame:rect]; - [button setImage:ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed( - IDR_ICON_PROFILES_EDIT_CAMERA).AsNSImage()]; - [button setImagePosition:NSImageOnly]; - [button setTarget:self]; - [button setAction:@selector(editPhoto:)]; - return button; -} @end // A custom text control that turns into a textfield for editing when clicked. @@ -772,6 +767,26 @@ - (void)drawRect:(NSRect)dirtyRect { } @end +// A custom dummy button that is used to clear focus from the bubble's controls. +@interface DummyWindowFocusButton : NSButton +@end + +@implementation DummyWindowFocusButton +// Ignore accessibility, as this is a placeholder button. +- (BOOL)accessibilityIsIgnored { + return YES; +} + +- (id)accessibilityAttributeValue:(NSString*)attribute { + return @[]; +} + +- (BOOL)canBecomeKeyView { + return false; +} + +@end + @interface ProfileChooserController () // Builds the profile chooser view. - (NSView*)buildProfileChooserView; @@ -1126,7 +1141,15 @@ - (void)initMenuContentsWithView:(profiles::BubbleViewMode)viewToDisplay { if (viewMode_ != profiles::BUBBLE_VIEW_MODE_PROFILE_CHOOSER) tutorialMode_ = profiles::TUTORIAL_MODE_NONE; + // Add a dummy, empty element so that we don't initially display any + // focus rings. + NSButton* dummyFocusButton = + [[[DummyWindowFocusButton alloc] initWithFrame:NSZeroRect] autorelease]; + [dummyFocusButton setNextKeyView:subView]; + [[self window] makeFirstResponder:dummyFocusButton]; + [contentView addSubview:subView]; + [contentView addSubview:dummyFocusButton]; SetWindowSize([self window], NSMakeSize(NSWidth([subView frame]), NSHeight([subView frame]))); } @@ -1670,7 +1693,8 @@ - (NSView*)createGuestProfileView { - (NSButton*)createOtherProfileView:(int)itemIndex { const AvatarMenu::Item& item = avatarMenu_->GetItemAt(itemIndex); - NSRect rect = NSMakeRect(0, 0, kFixedMenuWidth, kBlueButtonHeight); + NSRect rect = NSMakeRect( + 0, 0, kFixedMenuWidth, kBlueButtonHeight + kSmallVerticalSpacing); base::scoped_nsobject profileButton( [[BackgroundColorHoverButton alloc] initWithFrame:rect diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm index e253775477040..84b88582cd11b 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller_unittest.mm @@ -99,7 +99,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card, one separator and @@ -138,7 +138,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -166,7 +166,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // Three profiles means we should have one active card and a @@ -209,7 +209,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -240,7 +240,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSString* sortedNames[] = { @"Another Test", @"New Profile", @@ -272,7 +272,7 @@ void EnableFastUserSwitching() { switches::EnableNewAvatarMenuForTesting(CommandLine::ForCurrentProcess()); StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -301,7 +301,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -323,7 +323,7 @@ void EnableFastUserSwitching() { StartProfileChooserController(); NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; NSArray* activeCardSubviews = [[subviews objectAtIndex:2] subviews]; NSArray* activeCardLinks = [[activeCardSubviews objectAtIndex:0] subviews]; @@ -358,7 +358,7 @@ void EnableFastUserSwitching() { profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT]; NSArray* subviews = [[[controller() window] contentView] subviews]; - ASSERT_EQ(1U, [subviews count]); + ASSERT_EQ(2U, [subviews count]); subviews = [[subviews objectAtIndex:0] subviews]; // There should be one active card, one accounts container, two separators @@ -435,7 +435,7 @@ void EnableFastUserSwitching() { // Profile icon. NSView* activeProfileImage = [activeCardSubviews objectAtIndex:2]; - EXPECT_TRUE([activeProfileImage isKindOfClass:[NSImageView class]]); + EXPECT_TRUE([activeProfileImage isKindOfClass:[NSButton class]]); // Profile name. NSView* activeProfileName = [activeCardSubviews objectAtIndex:1]; @@ -451,3 +451,69 @@ void EnableFastUserSwitching() { EXPECT_EQ(@selector(hideAccountManagement:), [link action]); EXPECT_EQ(controller(), [link target]); } + +TEST_F(ProfileChooserControllerTest, SignedInProfileLockDisabled) { + switches::EnableNewProfileManagementForTesting( + CommandLine::ForCurrentProcess()); + // Sign in the first profile. + ProfileInfoCache* cache = testing_profile_manager()->profile_info_cache(); + cache->SetUserNameOfProfileAtIndex(0, base::ASCIIToUTF16(kEmail)); + cache->SetLocalAuthCredentialsOfProfileAtIndex(0, std::string()); + + StartProfileChooserController(); + NSArray* subviews = [[[controller() window] contentView] subviews]; + ASSERT_EQ(2U, [subviews count]); + subviews = [[subviews objectAtIndex:0] subviews]; + + // Three profiles means we should have one active card, one separator, one + // option buttons view and a lock view. We also have an update promo for the + // new avatar menu. + // TODO(noms): Enforcing 5U fails on the waterfall debug bots, but it's not + // reproducible anywhere else. + ASSERT_GE([subviews count], 4U); + + // There will be three buttons and two separators in the option buttons view. + NSArray* buttonSubviews = [[subviews objectAtIndex:0] subviews]; + ASSERT_EQ(5U, [buttonSubviews count]); + + // There should be a lock button. + NSButton* lockButton = + base::mac::ObjCCast([buttonSubviews objectAtIndex:0]); + ASSERT_TRUE(lockButton); + EXPECT_EQ(@selector(lockProfile:), [lockButton action]); + EXPECT_EQ(controller(), [lockButton target]); + EXPECT_FALSE([lockButton isEnabled]); +} + +TEST_F(ProfileChooserControllerTest, SignedInProfileLockEnabled) { + switches::EnableNewProfileManagementForTesting( + CommandLine::ForCurrentProcess()); + // Sign in the first profile. + ProfileInfoCache* cache = testing_profile_manager()->profile_info_cache(); + cache->SetUserNameOfProfileAtIndex(0, base::ASCIIToUTF16(kEmail)); + cache->SetLocalAuthCredentialsOfProfileAtIndex(0, "YourHashHere"); + + StartProfileChooserController(); + NSArray* subviews = [[[controller() window] contentView] subviews]; + ASSERT_EQ(2U, [subviews count]); + subviews = [[subviews objectAtIndex:0] subviews]; + + // Three profiles means we should have one active card, one separator, one + // option buttons view and a lock view. We also have an update promo for the + // new avatar menu. + // TODO(noms): Enforcing 5U fails on the waterfall debug bots, but it's not + // reproducible anywhere else. + ASSERT_GE([subviews count], 4U); + + // There will be three buttons and two separators in the option buttons view. + NSArray* buttonSubviews = [[subviews objectAtIndex:0] subviews]; + ASSERT_EQ(5U, [buttonSubviews count]); + + // There should be a lock button. + NSButton* lockButton = + base::mac::ObjCCast([buttonSubviews objectAtIndex:0]); + ASSERT_TRUE(lockButton); + EXPECT_EQ(@selector(lockProfile:), [lockButton action]); + EXPECT_EQ(controller(), [lockButton target]); + EXPECT_TRUE([lockButton isEnabled]); +}