[Mac] Add accessibility features to the new avatar menu

chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm

Index: chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
diff --git a/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm b/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
index 65024ab6b23c8b4839acc84e31de4e08c5b70cdd..42546b539a29346a37bbe283c012b881b6be3862 100644
--- a/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm
@@ -13,9 +13,12 @@
 #include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #include "components/signin/core/browser/signin_error_controller.h"
+#include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #import "ui/base/cocoa/appkit_utils.h"
 #import "ui/base/cocoa/hover_image_button.h"
+#include "ui/base/l10n/l10n_util.h"

[groby-ooo-7-16, 2014/08/15 00:38:35]

Do you need this?

[noms (inactive), 2014/08/15 19:26:47]

Done.

+#include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/nine_image_painter_factory.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image_skia_operations.h"
@@ -146,6 +149,7 @@ NSImage* GetImageFromResourceID(int resourceId) {
 @interface AvatarButtonController (Private)
 - (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent;
 - (void)updateErrorStatus:(BOOL)hasError;
+- (void)updateAccessibilityValue:(BOOL)hasError;
 - (void)dealloc;
 - (void)themeDidChangeNotification:(NSNotification*)aNotification;
 @end
@@ -189,6 +193,8 @@ NSImage* GetImageFromResourceID(int resourceId) {
     [button_ setAction:@selector(buttonClicked:)];
 
     [self updateAvatarButtonAndLayoutParent:NO];
+    [self updateAccessibilityValue:
+        errorController && errorController->HasError()];
 
     NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
     [center addObserver:self
@@ -264,6 +270,21 @@ NSImage* GetImageFromResourceID(int resourceId) {
 - (void)updateErrorStatus:(BOOL)hasError {
   [[button_ cell] setHasError:hasError];
   [self updateAvatarButtonAndLayoutParent:YES];
+  [self updateAccessibilityValue:hasError];
+}
+
+- (void)updateAccessibilityValue:(BOOL)hasError {

[groby-ooo-7-16, 2014/08/15 00:38:35]

That's a horrible name, sorry. How about setAccessibleAccountButtonTitle, or
something that makes clear _what_ value to set?

[noms (inactive), 2014/08/15 19:26:47]

Acknowledged.

+  base::string16 buttonTitle = base::SysNSStringToUTF16([button_ title]);

[groby-ooo-7-16, 2014/08/15 00:38:35]

Silly question - why get this outside the if(), since that's the only place it's
used?

[noms (inactive), 2014/08/15 19:26:47]

No good reason

On 2014/08/15 00:38:35, groby wrote:

+  if (hasError) {
+    [[button_ cell]
+        accessibilitySetOverrideValue:l10n_util::GetNSStringF(

[groby-ooo-7-16, 2014/08/15 00:38:35]

Why not do all this magic in the cell - that way it's guaranteed to happen when
you change the error status?

[noms (inactive), 2014/08/15 19:26:47]

Hmm, because the cell didn't have the title but that was not actually a thing.
Done!

On 2014/08/15 00:38:35, groby wrote:

+            IDS_PROFILES_ACCOUNT_BUTTON_AUTH_ERROR_ACCESSIBLE_NAME, buttonTitle)
+                         forAttribute:NSAccessibilityTitleAttribute];
+  } else {
+    [[button_ cell]
+        accessibilitySetOverrideValue:[button_ title]
+                         forAttribute:NSAccessibilityTitleAttribute];
+  }
 }
 
 @end