Commit 780c37b8 authored by Yi Su's avatar Yi Su Committed by Commit Bot

Revert "[ios] Fix BadgeViewController interpretation of badgeState"

This reverts commit 4fcce6d7.

Reason for revert: SCBadgeTestCase.testBadgesVisible is failing consistently after this CL:
https://ci.chromium.org/p/chrome/builders/ci/iphone-simulator/5469

Original change's description:
> [ios] Fix BadgeViewController interpretation of badgeState
> 
> Since BadgeState is a mask, a direct equality comparison to
> BadgeState values for the BadgeItem properties is not the correct
> way to assess if a badge is accepted.
> 
> To add testing support for this case, accepted a11y identifiers
> for BadgeButtons are created, and they will be properly set
> in setAccepted: in BadgeButton.
> 
> Bug: 976901, 1016793
> Change-Id: Id17eb82e0764fbbf80b871df0b974fe148c2339f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1872938
> Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
> Reviewed-by: Peter Lee <pkl@chromium.org>
> Reviewed-by: Sergio Collazos <sczs@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#709037}

TBR=pkl@chromium.org,sczs@chromium.org,thegreenfrog@chromium.org

Change-Id: I7cc7f67cbb24a962388c2079a71e9e168c32fdfb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 976901, 1016793
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880029Reviewed-by: default avatarYi Su <mrsuyi@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709406}
parent dcbbe562
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
#import "ios/chrome/browser/ui/badges/badge_button.h" #import "ios/chrome/browser/ui/badges/badge_button.h"
#import "base/logging.h"
#import "ios/chrome/browser/ui/badges/badge_constants.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/common/colors/semantic_color_names.h" #import "ios/chrome/common/colors/semantic_color_names.h"
...@@ -47,7 +45,6 @@ const CGFloat kButtonCircularCornerRadiusDivisor = 2.0; ...@@ -47,7 +45,6 @@ const CGFloat kButtonCircularCornerRadiusDivisor = 2.0;
self.accepted = accepted; self.accepted = accepted;
void (^changeTintColor)() = ^{ void (^changeTintColor)() = ^{
self.tintColor = accepted ? nil : [UIColor colorNamed:kToolbarButtonColor]; self.tintColor = accepted ? nil : [UIColor colorNamed:kToolbarButtonColor];
self.accessibilityIdentifier = [self getAccessibilityIdentifier:accepted];
}; };
if (animated) { if (animated) {
[UIView animateWithDuration:kButtonAnimationDuration [UIView animateWithDuration:kButtonAnimationDuration
...@@ -76,31 +73,6 @@ const CGFloat kButtonCircularCornerRadiusDivisor = 2.0; ...@@ -76,31 +73,6 @@ const CGFloat kButtonCircularCornerRadiusDivisor = 2.0;
#pragma mark - Private #pragma mark - Private
- (NSString*)getAccessibilityIdentifier:(BOOL)accepted {
switch (self.badgeType) {
case BadgeType::kBadgeTypeNone:
NOTREACHED() << "A badge should not have kBadgeTypeNone";
return nil;
case BadgeType::kBadgeTypePasswordSave:
return accepted ? kBadgeButtonSavePasswordAcceptedAccessibilityIdentifier
: kBadgeButtonSavePasswordAccessibilityIdentifier;
case BadgeType::kBadgeTypePasswordUpdate:
return accepted
? kBadgeButtonUpdatePasswordAccpetedAccessibilityIdentifier
: kBadgeButtonUpdatePasswordAccessibilityIdentifier;
case BadgeType::kBadgeTypeIncognito:
return kBadgeButtonIncognitoAccessibilityIdentifier;
case BadgeType::kBadgeTypeOverflow:
return kBadgeButtonOverflowAccessibilityIdentifier;
case BadgeType::kBadgeTypeSaveCard:
return accepted ? kBadgeButtonSaveCardAcceptedAccessibilityIdentifier
: kBadgeButtonSaveCardAccessibilityIdentifier;
case BadgeType::kBadgeTypeTranslate:
return accepted ? kBadgeButtonTranslateAcceptedAccessibilityIdentifier
: kBadgeButtonTranslateAccessibilityIdentifier;
}
}
- (void)configureImage { - (void)configureImage {
if (self.fullScreenOn && self.fullScreenImage) { if (self.fullScreenOn && self.fullScreenImage) {
[self setImage:self.fullScreenImage forState:UIControlStateNormal]; [self setImage:self.fullScreenImage forState:UIControlStateNormal];
......
...@@ -9,16 +9,11 @@ ...@@ -9,16 +9,11 @@
// A11y identifiers so that automation can tap on BadgeButtons // A11y identifiers so that automation can tap on BadgeButtons
extern NSString* const kBadgeButtonSavePasswordAccessibilityIdentifier; extern NSString* const kBadgeButtonSavePasswordAccessibilityIdentifier;
extern NSString* const kBadgeButtonSavePasswordAcceptedAccessibilityIdentifier;
extern NSString* const kBadgeButtonUpdatePasswordAccessibilityIdentifier; extern NSString* const kBadgeButtonUpdatePasswordAccessibilityIdentifier;
extern NSString* const
kBadgeButtonUpdatePasswordAccpetedAccessibilityIdentifier;
extern NSString* const kBadgeButtonIncognitoAccessibilityIdentifier; extern NSString* const kBadgeButtonIncognitoAccessibilityIdentifier;
extern NSString* const kBadgeButtonOverflowAccessibilityIdentifier; extern NSString* const kBadgeButtonOverflowAccessibilityIdentifier;
extern NSString* const kBadgeButtonSaveCardAccessibilityIdentifier; extern NSString* const kBadgeButtonSaveCardAccessibilityIdentifier;
extern NSString* const kBadgeButtonSaveCardAcceptedAccessibilityIdentifier;
extern NSString* const kBadgeButtonTranslateAccessibilityIdentifier; extern NSString* const kBadgeButtonTranslateAccessibilityIdentifier;
extern NSString* const kBadgeButtonTranslateAcceptedAccessibilityIdentifier;
// A11y identifier for the Badge Popup Menu Table View. // A11y identifier for the Badge Popup Menu Table View.
extern NSString* const kBadgePopupMenuTableViewAccessibilityIdentifier; extern NSString* const kBadgePopupMenuTableViewAccessibilityIdentifier;
......
...@@ -11,15 +11,9 @@ ...@@ -11,15 +11,9 @@
NSString* const kBadgeButtonSavePasswordAccessibilityIdentifier = NSString* const kBadgeButtonSavePasswordAccessibilityIdentifier =
@"badgeButtonSavePasswordAXID"; @"badgeButtonSavePasswordAXID";
NSString* const kBadgeButtonSavePasswordAcceptedAccessibilityIdentifier =
@"badgeButtonSavePasswordAcceptedAXID";
NSString* const kBadgeButtonUpdatePasswordAccessibilityIdentifier = NSString* const kBadgeButtonUpdatePasswordAccessibilityIdentifier =
@"badgeButtonUpdatePasswordAXID"; @"badgeButtonUpdatePasswordAXID";
NSString* const kBadgeButtonUpdatePasswordAccpetedAccessibilityIdentifier =
@"badgeButtonUpdatePasswordAcceptedAXID";
NSString* const kBadgeButtonIncognitoAccessibilityIdentifier = NSString* const kBadgeButtonIncognitoAccessibilityIdentifier =
@"badgeButtonIncognitoAXID"; @"badgeButtonIncognitoAXID";
...@@ -29,15 +23,9 @@ NSString* const kBadgeButtonOverflowAccessibilityIdentifier = ...@@ -29,15 +23,9 @@ NSString* const kBadgeButtonOverflowAccessibilityIdentifier =
NSString* const kBadgeButtonSaveCardAccessibilityIdentifier = NSString* const kBadgeButtonSaveCardAccessibilityIdentifier =
@"badgeButtonSaveCardAXID"; @"badgeButtonSaveCardAXID";
NSString* const kBadgeButtonSaveCardAcceptedAccessibilityIdentifier =
@"badgeButtonSaveCardAcceptedAXID";
NSString* const kBadgeButtonTranslateAccessibilityIdentifier = NSString* const kBadgeButtonTranslateAccessibilityIdentifier =
@"badgeButtonTranslateAXID"; @"badgeButtonTranslateAXID";
NSString* const kBadgeButtonTranslateAcceptedAccessibilityIdentifier =
@"badgeButtonTranslateAcceptedAXID";
NSString* const kBadgePopupMenuTableViewAccessibilityIdentifier = NSString* const kBadgePopupMenuTableViewAccessibilityIdentifier =
@"badgePopupMenuOverflowAXID"; @"badgePopupMenuOverflowAXID";
......
...@@ -89,7 +89,7 @@ const CGFloat kUnreadIndicatorViewHeight = 6.0; ...@@ -89,7 +89,7 @@ const CGFloat kUnreadIndicatorViewHeight = 6.0;
if (displayedBadgeItem) { if (displayedBadgeItem) {
BadgeButton* newButton = [self.buttonFactory BadgeButton* newButton = [self.buttonFactory
getBadgeButtonForBadgeType:displayedBadgeItem.badgeType]; getBadgeButtonForBadgeType:displayedBadgeItem.badgeType];
[newButton setAccepted:displayedBadgeItem.badgeState & BadgeStateAccepted [newButton setAccepted:displayedBadgeItem.badgeState == BadgeStateAccepted
animated:NO]; animated:NO];
self.displayedBadge = newButton; self.displayedBadge = newButton;
} }
...@@ -116,12 +116,12 @@ const CGFloat kUnreadIndicatorViewHeight = 6.0; ...@@ -116,12 +116,12 @@ const CGFloat kUnreadIndicatorViewHeight = 6.0;
if (self.displayedBadge && if (self.displayedBadge &&
self.displayedBadge.badgeType == displayedBadgeItem.badgeType) { self.displayedBadge.badgeType == displayedBadgeItem.badgeType) {
[self.displayedBadge [self.displayedBadge
setAccepted:displayedBadgeItem.badgeState & BadgeStateAccepted setAccepted:displayedBadgeItem.badgeState == BadgeStateAccepted
animated:YES]; animated:YES];
} else { } else {
BadgeButton* newButton = [self.buttonFactory BadgeButton* newButton = [self.buttonFactory
getBadgeButtonForBadgeType:displayedBadgeItem.badgeType]; getBadgeButtonForBadgeType:displayedBadgeItem.badgeType];
[newButton setAccepted:displayedBadgeItem.badgeState & BadgeStateAccepted [newButton setAccepted:displayedBadgeItem.badgeState == BadgeStateAccepted
animated:NO]; animated:NO];
self.displayedBadge = newButton; self.displayedBadge = newButton;
} }
......
...@@ -17,7 +17,9 @@ ...@@ -17,7 +17,9 @@
// successfully. If it returns NO no Modal should be presented. // successfully. If it returns NO no Modal should be presented.
- (BOOL)configureModalViewController; - (BOOL)configureModalViewController;
// Returns YES if the Infobar Accept action was completed successfully. // Returns YES if the Infobar action was completed successfully. Useful for
// Infobar actions that finish asynchronously to delay marking the Infobar as
// accepted.
- (BOOL)isInfobarAccepted; - (BOOL)isInfobarAccepted;
// Performs any actions related to an Infobar Banner presentation. // Performs any actions related to an Infobar Banner presentation.
......
...@@ -65,7 +65,8 @@ ...@@ -65,7 +65,8 @@
- (void)translateInfoBarDelegate:(translate::TranslateInfoBarDelegate*)delegate - (void)translateInfoBarDelegate:(translate::TranslateInfoBarDelegate*)delegate
didChangeTranslateStep:(translate::TranslateStep)step didChangeTranslateStep:(translate::TranslateStep)step
withErrorType:(translate::TranslateErrors::Type)errorType { withErrorType:(translate::TranslateErrors::Type)errorType {
// TODO(crbug.com/1014959): implement // TODO(crbug.com/1014959): Update currentStep and call
// infobarWasAccepted:forWebState: if translate finished.
} }
- (BOOL)translateInfoBarDelegateDidDismissWithoutInteraction: - (BOOL)translateInfoBarDelegateDidDismissWithoutInteraction:
......
...@@ -9,10 +9,7 @@ ...@@ -9,10 +9,7 @@
// A11y identifier for button that will replace the displayed badge with the // A11y identifier for button that will replace the displayed badge with the
// overflow badge button. // overflow badge button.
extern NSString* const kSCShowOverflowDisplayedBadgeButton; extern NSString* const kSCDisplayedBadgeToggleButton;
// A11y identifier for button that will show an accepted displayed button.
extern NSString* const kSCShowAcceptedDisplayedBadgeButton;
@interface SCBadgeCoordinator : NSObject <NavigationCoordinator> @interface SCBadgeCoordinator : NSObject <NavigationCoordinator>
......
...@@ -20,11 +20,8 @@ ...@@ -20,11 +20,8 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
NSString* const kSCShowOverflowDisplayedBadgeButton = NSString* const kSCDisplayedBadgeToggleButton =
@"kSCShowOverflowDisplayedBadgeButtonAXID"; @"kSCDisplayedBadgeToggleButtonAXID";
NSString* const kSCShowAcceptedDisplayedBadgeButton =
@"kSCShowAcceptedDisplayedBadgeButtonAXID";
@interface BadgeContainerViewController : UIViewController @interface BadgeContainerViewController : UIViewController
@property(nonatomic, strong) BadgeViewController* centeredChildViewController; @property(nonatomic, strong) BadgeViewController* centeredChildViewController;
...@@ -57,27 +54,13 @@ NSString* const kSCShowAcceptedDisplayedBadgeButton = ...@@ -57,27 +54,13 @@ NSString* const kSCShowAcceptedDisplayedBadgeButton =
constraintEqualToConstant:100] constraintEqualToConstant:100]
]]; ]];
UIButton* showAcceptedBadgeButton = UIButton* button = [UIButton buttonWithType:UIButtonTypeSystem];
[UIButton buttonWithType:UIButtonTypeSystem]; button.accessibilityIdentifier = kSCDisplayedBadgeToggleButton;
showAcceptedBadgeButton.accessibilityIdentifier = [button setTitle:@"Show Overflow badge" forState:UIControlStateNormal];
kSCShowAcceptedDisplayedBadgeButton; [button addTarget:self
[showAcceptedBadgeButton setTitle:@"Show Accepted Badge" action:@selector(addSecondBadge:)
forState:UIControlStateNormal]; forControlEvents:UIControlEventTouchUpInside];
[showAcceptedBadgeButton addTarget:self [stackView addArrangedSubview:button];
action:@selector(showAcceptedDisplayedBadge)
forControlEvents:UIControlEventTouchUpInside];
[stackView addArrangedSubview:showAcceptedBadgeButton];
UIButton* showOverflowBadgeButton =
[UIButton buttonWithType:UIButtonTypeSystem];
showOverflowBadgeButton.accessibilityIdentifier =
kSCShowOverflowDisplayedBadgeButton;
[showOverflowBadgeButton setTitle:@"Show Overflow badge"
forState:UIControlStateNormal];
[showOverflowBadgeButton addTarget:self
action:@selector(addSecondBadge:)
forControlEvents:UIControlEventTouchUpInside];
[stackView addArrangedSubview:showOverflowBadgeButton];
UIView* containerView = self.view; UIView* containerView = self.view;
containerView.backgroundColor = [UIColor whiteColor]; containerView.backgroundColor = [UIColor whiteColor];
...@@ -87,17 +70,6 @@ NSString* const kSCShowAcceptedDisplayedBadgeButton = ...@@ -87,17 +70,6 @@ NSString* const kSCShowAcceptedDisplayedBadgeButton =
initWithBadgeType:BadgeType::kBadgeTypeIncognito]; initWithBadgeType:BadgeType::kBadgeTypeIncognito];
InfobarBadgeModel* passwordBadgeItem = [[InfobarBadgeModel alloc] InfobarBadgeModel* passwordBadgeItem = [[InfobarBadgeModel alloc]
initWithInfobarType:InfobarType::kInfobarTypePasswordSave]; initWithInfobarType:InfobarType::kInfobarTypePasswordSave];
passwordBadgeItem.badgeState = BadgeStateRead;
[self.consumer setupWithDisplayedBadge:passwordBadgeItem
fullScreenBadge:incognitoItem];
}
- (void)showAcceptedDisplayedBadge {
BadgeStaticItem* incognitoItem = [[BadgeStaticItem alloc]
initWithBadgeType:BadgeType::kBadgeTypeIncognito];
InfobarBadgeModel* passwordBadgeItem = [[InfobarBadgeModel alloc]
initWithInfobarType:InfobarType::kInfobarTypePasswordSave];
passwordBadgeItem.badgeState = BadgeStateRead | BadgeStateAccepted;
[self.consumer setupWithDisplayedBadge:passwordBadgeItem [self.consumer setupWithDisplayedBadge:passwordBadgeItem
fullScreenBadge:incognitoItem]; fullScreenBadge:incognitoItem];
} }
......
...@@ -43,23 +43,11 @@ using ::showcase_utils::Close; ...@@ -43,23 +43,11 @@ using ::showcase_utils::Close;
kBadgeButtonSavePasswordAccessibilityIdentifier), kBadgeButtonSavePasswordAccessibilityIdentifier),
grey_sufficientlyVisible(), nil)] grey_sufficientlyVisible(), nil)]
assertWithMatcher:grey_sufficientlyVisible()]; assertWithMatcher:grey_sufficientlyVisible()];
// Tap on button to show the accepted badge.
[[EarlGrey selectElementWithMatcher:grey_accessibilityID(
kSCShowAcceptedDisplayedBadgeButton)]
performAction:grey_tap()];
[[EarlGrey [[EarlGrey
selectElementWithMatcher: selectElementWithMatcher:
grey_allOf( grey_allOf(grey_accessibilityID(
grey_accessibilityID( kBadgeButtonSavePasswordAccessibilityIdentifier),
kBadgeButtonSavePasswordAcceptedAccessibilityIdentifier), grey_sufficientlyVisible(), nil)]
grey_sufficientlyVisible(), nil)]
assertWithMatcher:grey_sufficientlyVisible()];
[[EarlGrey selectElementWithMatcher:
grey_allOf(grey_accessibilityID(
kBadgeButtonIncognitoAccessibilityIdentifier),
grey_sufficientlyVisible(), nil)]
assertWithMatcher:grey_sufficientlyVisible()]; assertWithMatcher:grey_sufficientlyVisible()];
} }
...@@ -68,7 +56,7 @@ using ::showcase_utils::Close; ...@@ -68,7 +56,7 @@ using ::showcase_utils::Close;
- (void)testOverflowbadge { - (void)testOverflowbadge {
// Tap on button to show the overflow badge. // Tap on button to show the overflow badge.
[[EarlGrey selectElementWithMatcher:grey_accessibilityID( [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
kSCShowOverflowDisplayedBadgeButton)] kSCDisplayedBadgeToggleButton)]
performAction:grey_tap()]; performAction:grey_tap()];
// Assert that overflow badge and the unread indicator is shown and tap on it. // Assert that overflow badge and the unread indicator is shown and tap on it.
...@@ -99,7 +87,7 @@ using ::showcase_utils::Close; ...@@ -99,7 +87,7 @@ using ::showcase_utils::Close;
// badge is sufficient here. Assert that the unread indicator is not there // badge is sufficient here. Assert that the unread indicator is not there
// anymore. // anymore.
[[EarlGrey selectElementWithMatcher:grey_accessibilityID( [[EarlGrey selectElementWithMatcher:grey_accessibilityID(
kSCShowOverflowDisplayedBadgeButton)] kSCDisplayedBadgeToggleButton)]
performAction:grey_tap()]; performAction:grey_tap()];
[[EarlGrey selectElementWithMatcher: [[EarlGrey selectElementWithMatcher:
grey_allOf(grey_accessibilityID( grey_allOf(grey_accessibilityID(
......
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