Provide Copy button for username in password settings

ios/chrome/browser/ui/settings/password_details_collection_view_controller.mm

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/chrome/browser/ui/settings/password_details_collection_view_controller.h"
 
 #include "base/mac/foundation_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/affiliation_utils.h"
@@ skipping 21 matching lines @@
 namespace {
 
 typedef NS_ENUM(NSInteger, SectionIdentifier) {
   SectionIdentifierUsername = kSectionIdentifierEnumZero,
   SectionIdentifierPassword,
 };
 
 typedef NS_ENUM(NSInteger, ItemType) {
   ItemTypeHeader = kItemTypeEnumZero,
   ItemTypeUsername,
+  ItemTypeCopyUsername,
   ItemTypePassword,
   ItemTypeShowHide,
-  ItemTypeCopy,
+  ItemTypeCopyPassword,
   ItemTypeDelete,
 };
 
 }  // namespace
 
 @interface PasswordDetailsCollectionViewController () {
   // The username to which the saved password belongs.
   NSString* _username;
   // The saved password.
   NSString* _password;
@@ skipping 70 matching lines @@
       l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_USERNAME);
   usernameHeader.textColor = [[MDCPalette greyPalette] tint500];
   [model setHeader:usernameHeader
       forSectionWithIdentifier:SectionIdentifierUsername];
   PasswordDetailsItem* usernameItem =
       [[PasswordDetailsItem alloc] initWithType:ItemTypeUsername];
   usernameItem.text = _username;
   usernameItem.showingText = YES;
   [model addItem:usernameItem
       toSectionWithIdentifier:SectionIdentifierUsername];
+  [model addItem:[self usernameCopyButtonItem]
+      toSectionWithIdentifier:SectionIdentifierUsername];
 
   [model addSectionWithIdentifier:SectionIdentifierPassword];
   CollectionViewTextItem* passwordHeader =
       [[CollectionViewTextItem alloc] initWithType:ItemTypeHeader];
   passwordHeader.text =
       l10n_util::GetNSString(IDS_IOS_SHOW_PASSWORD_VIEW_PASSWORD);
   passwordHeader.textColor = [[MDCPalette greyPalette] tint500];
   [model setHeader:passwordHeader
       forSectionWithIdentifier:SectionIdentifierPassword];
   _passwordItem = [[PasswordDetailsItem alloc] initWithType:ItemTypePassword];
@@ skipping 11 matching lines @@
   [model addItem:[self deletePasswordButtonItem]
       toSectionWithIdentifier:SectionIdentifierPassword];
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
 }
 
 #pragma mark - Items
 
+- (CollectionViewItem*)usernameCopyButtonItem {
+  CollectionViewTextItem* item =
+      [[CollectionViewTextItem alloc] initWithType:ItemTypeCopyUsername];
+  item.text = l10n_util::GetNSString(IDS_IOS_SETTINGS_USERNAME_COPY_BUTTON);

[lpromero, 2017/05/02 15:04:52]

This might not be sufficient for accessibility. I suggest you set the
accessibilityLabel to a concatenation of "Copy" and "Username", or make an
entirely new string.

[vabr (Chromium), 2017/05/02 15:29:09]

Oh, so if I set accessibilityLabel here to "Copy Username", that will override
the creation of the accessibility label from text? When looking at
https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/collection_view/ce...
it seemed to me that there is no way to decouple the text from the acecssibility
label.

My issue with changing the text would be that UI-wise it makes sense not to
repeat "Username" if that's the header of the section.

And given that EGTest can express the correct "Copy" matcher relative to the
headers, I was hoping VoiceOver could also be helpful in this respect, in that
the user would hear, e.g., "Username", <username itself>, "Copy", when
progressing through the UI. Would that not work for accessibility?

[lpromero, 2017/05/02 16:27:20]

Indeed. This is an issue. The collection view text item should first see if
there is an accessibility label, and only fallback to the code you linked.
Can you file a bug and maybe send a CL to fix it?
The problem is that this namespacing is lost when a VoiceOver user is focused on
"Copy". The user could have come from the bottom, or from the top, but also by
moving their finger on the screen.
Visually, having only "Copy" is fine, but not for visually impaired users.
As explained above, it's not evidence that users will parse the UI in this
order. But no need to change the EG tests, they are fine as is.

+  item.textColor = [[MDCPalette cr_bluePalette] tint500];
+  item.accessibilityTraits |= UIAccessibilityTraitButton;
+  return item;
+}
+
 - (CollectionViewItem*)passwordCopyButtonItem {
   CollectionViewTextItem* item =
-      [[CollectionViewTextItem alloc] initWithType:ItemTypeCopy];
+      [[CollectionViewTextItem alloc] initWithType:ItemTypeCopyPassword];
   item.text = l10n_util::GetNSString(IDS_IOS_SETTINGS_PASSWORD_COPY_BUTTON);
   item.textColor = [[MDCPalette cr_bluePalette] tint500];
   item.accessibilityTraits |= UIAccessibilityTraitButton;
   return item;
 }
 
 - (CollectionViewItem*)showHidePasswordButtonItem {
   CollectionViewTextItem* item =
       [[CollectionViewTextItem alloc] initWithType:ItemTypeShowHide];
   item.text = [self showHideButtonText];
   item.textColor = [[MDCPalette cr_bluePalette] tint500];
   item.accessibilityTraits |= UIAccessibilityTraitButton;
   return item;
 }
 
 - (CollectionViewItem*)deletePasswordButtonItem {
   CollectionViewTextItem* item =
       [[CollectionViewTextItem alloc] initWithType:ItemTypeDelete];
   item.text = l10n_util::GetNSString(IDS_IOS_SETTINGS_PASSWORD_DELETE_BUTTON);
   item.textColor = [[MDCPalette cr_redPalette] tint500];
   item.accessibilityTraits |= UIAccessibilityTraitButton;
   return item;
 }
 
 #pragma mark - Actions
 
+- (void)copyUsername {
+  UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
+  generalPasteboard.string = _username;
+  [self showCopyResultToast:l10n_util::GetNSString(
+                                IDS_IOS_SETTINGS_USERNAME_WAS_COPIED_MESSAGE)];
+}
+
 - (NSString*)showHideButtonText {
   if (_plainTextPasswordShown) {
     return l10n_util::GetNSString(IDS_IOS_SETTINGS_PASSWORD_HIDE_BUTTON);
   }
   return l10n_util::GetNSString(IDS_IOS_SETTINGS_PASSWORD_SHOW_BUTTON);
 }
 
 // Changes the text on the Show/Hide button appropriately according to
 // |_plainTextPasswordShown|.
 - (void)toggleShowHideButton {
@@ skipping 50 matching lines @@
 }
 
 - (void)copyPassword {
   // If the password is displayed in plain text, there is no need to
   // re-authenticate the user when copying the password because they are already
   // granted access to it.
   if (_plainTextPasswordShown) {
     UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
     generalPasteboard.string = _password;
     TriggerHapticFeedbackForNotification(UINotificationFeedbackTypeSuccess);
-    [self showCopyPasswordResultToast:
-              l10n_util::GetNSString(
-                  IDS_IOS_SETTINGS_PASSWORD_WAS_COPIED_MESSAGE)];
+    [self
+        showCopyResultToast:l10n_util::GetNSString(
+                                IDS_IOS_SETTINGS_PASSWORD_WAS_COPIED_MESSAGE)];
   } else if ([_weakReauthenticationModule canAttemptReauth]) {
     __weak PasswordDetailsCollectionViewController* weakSelf = self;
     void (^copyPasswordHandler)(BOOL) = ^(BOOL success) {
       PasswordDetailsCollectionViewController* strongSelf = weakSelf;
       if (!strongSelf)
         return;
       if (success) {
         UIPasteboard* generalPasteboard = [UIPasteboard generalPasteboard];
         generalPasteboard.string = strongSelf->_password;
         TriggerHapticFeedbackForNotification(UINotificationFeedbackTypeSuccess);
-        [strongSelf showCopyPasswordResultToast:
+        [strongSelf showCopyResultToast:
                         l10n_util::GetNSString(
                             IDS_IOS_SETTINGS_PASSWORD_WAS_COPIED_MESSAGE)];
       } else {
         TriggerHapticFeedbackForNotification(UINotificationFeedbackTypeError);
-        [strongSelf showCopyPasswordResultToast:
+        [strongSelf showCopyResultToast:
                         l10n_util::GetNSString(
                             IDS_IOS_SETTINGS_PASSWORD_WAS_NOT_COPIED_MESSAGE)];
       }
     };
     [_weakReauthenticationModule
         attemptReauthWithLocalizedReason:
             l10n_util::GetNSString(IDS_IOS_SETTINGS_PASSWORD_REAUTH_REASON_COPY)
                                  handler:copyPasswordHandler];
   }
 }
 
-- (void)showCopyPasswordResultToast:(NSString*)message {
+- (void)showCopyResultToast:(NSString*)message {
   // TODO(crbug.com/159166): Route this through some delegate API to be able
   // to mock it in the unittest, and avoid having an EGTest just for that?
   MDCSnackbarMessage* copyPasswordResultMessage =
       [MDCSnackbarMessage messageWithText:message];
   [MDCSnackbarManager showMessage:copyPasswordResultMessage];
 }
 
 - (void)deletePassword {
   [_weakDelegate deletePassword:_passwordForm];
 }
 
 #pragma mark - UICollectionViewDelegate
 
 - (void)collectionView:(UICollectionView*)collectionView
     didSelectItemAtIndexPath:(NSIndexPath*)indexPath {
   [super collectionView:collectionView didSelectItemAtIndexPath:indexPath];
   NSInteger itemType =
       [self.collectionViewModel itemTypeForIndexPath:indexPath];
   switch (itemType) {
+    case ItemTypeCopyUsername:
+      [self copyUsername];
+      break;
     case ItemTypeShowHide:
       if (_plainTextPasswordShown) {
         [self hidePassword];
       } else {
         [self showPassword];
       }
       break;
-    case ItemTypeCopy:
+    case ItemTypeCopyPassword:
       [self copyPassword];
       break;
     case ItemTypeDelete:
       [self deletePassword];
       break;
     default:
       break;
   }
 }
 
@@ skipping 15 matching lines @@
 }
 
 #pragma mark - ForTesting
 
 - (void)setReauthenticationModule:
     (id<ReauthenticationProtocol>)reauthenticationModule {
   _weakReauthenticationModule = reauthenticationModule;
 }
 
 @end