Skip to content

Commit

Permalink
[ios] Fix a11y when thumbstrip on.
Browse files Browse the repository at this point in the history
One of my previous code path change caused skipping of setting
accessibilityViewIsModal on browser view. Changed the animatee in
TabGridViewController to set both correctly, at the same time.

Include a test patch renaming properties not uploaded in previous cl.

Bug: 1311989
Change-Id: I1b2f1e31a8fe983ce3c3ccf1d32617eadb4b44d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3570546
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Commit-Queue: David Jean <djean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990921}
  • Loading branch information
David Jean authored and Chromium LUCI CQ committed Apr 11, 2022
1 parent d661739 commit ac2fb51
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ - (void)installThumbStrip {
[self.baseViewController addChildViewController:self.bvcContainer];
self.bvcContainer.view.frame = self.baseViewController.view.bounds;
[self.baseViewController.view addSubview:self.bvcContainer.view];
self.bvcContainer.view.accessibilityViewIsModal = YES;
[self.bvcContainer didMoveToParentViewController:self.baseViewController];
}

Expand Down Expand Up @@ -943,6 +942,10 @@ - (void)dismissBVC {
completion:nil];
}

- (void)setBVCAccessibilityViewModal:(BOOL)modal {
self.bvcContainer.view.accessibilityViewIsModal = modal;
}

- (void)openSearchResultsPageForSearchText:(NSString*)searchText {
TemplateURLService* templateURLService =
ios::TemplateURLServiceFactory::GetForBrowserState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ void SetUp() override {
incognitoBrowser:incognito_browser_.get()];
coordinator_.animationsDisabledForTesting = YES;

regular_thumbStrip_supporting_ = [[StubThumbStripSupporting alloc] init];
incognito_thumbStrip_supporting_ = [[StubThumbStripSupporting alloc] init];
coordinator_.regularThumbStripSupporting = regular_thumbStrip_supporting_;
regular_thumb_strip_supporting_ = [[StubThumbStripSupporting alloc] init];
incognito_thumb_strip_supporting_ = [[StubThumbStripSupporting alloc] init];
coordinator_.regularThumbStripSupporting = regular_thumb_strip_supporting_;
coordinator_.incognitoThumbStripSupporting =
incognito_thumbStrip_supporting_;
incognito_thumb_strip_supporting_;

// TabGridCoordinator will make its view controller the root, so stash the
// original root view controller before starting |coordinator_|.
Expand All @@ -192,7 +192,7 @@ void TearDown() override {
}

UIViewController* GetBaseViewController() {
if (regular_thumbStrip_supporting_.thumbStripEnabled) {
if (regular_thumb_strip_supporting_.thumbStripEnabled) {
return base::mac::ObjCCastStrict<UIViewController>(
coordinator_.bvcContainer);
} else {
Expand Down Expand Up @@ -238,8 +238,8 @@ void TearDown() override {
base::ScopedMockClockOverride scoped_clock_;

// Thumbstrip supporting stubs.
StubThumbStripSupporting* regular_thumbStrip_supporting_;
StubThumbStripSupporting* incognito_thumbStrip_supporting_;
StubThumbStripSupporting* regular_thumb_strip_supporting_;
StubThumbStripSupporting* incognito_thumb_strip_supporting_;

// PopupMenuCoordinator nedded for Thumbstrip support.
PopupMenuCoordinator* regular_popup_menu_coordinator_;
Expand Down Expand Up @@ -282,8 +282,9 @@ void TearDown() override {
completion:nil];
EXPECT_EQ(normal_tab_view_controller_, coordinator_.activeViewController);

if (!regular_thumbStrip_supporting_.thumbStripEnabled) {
// Showing the TabSwitcher again will make it active.
if (!regular_thumb_strip_supporting_.thumbStripEnabled) {
// Showing the TabSwitcher again will make it active, except with
// thumbstrip where the normal_tab_view_controller_ remains active.
[coordinator_ showTabGrid];
bool tab_switcher_active = base::test::ios::WaitUntilConditionOrTimeout(
base::test::ios::kWaitForUIElementTimeout, ^bool {
Expand Down Expand Up @@ -352,7 +353,7 @@ void TearDown() override {
return completion_handler_was_called;
});
ASSERT_TRUE(completion_handler_was_called);
if (!regular_thumbStrip_supporting_.thumbStripEnabled) {
if (!regular_thumb_strip_supporting_.thumbStripEnabled) {
// Thumbstrip doesn't call delegate.
EXPECT_TRUE(delegate_.didEndCalled);
}
Expand All @@ -370,7 +371,7 @@ void TearDown() override {
return completion_handler_was_called;
});
ASSERT_TRUE(completion_handler_was_called);
if (!regular_thumbStrip_supporting_.thumbStripEnabled) {
if (!regular_thumb_strip_supporting_.thumbStripEnabled) {
// Thumbstrip doesn't call delegate.
EXPECT_FALSE(delegate_.didEndCalled);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ enum class TabGridPageConfiguration {
// Opens a link when the user clicks on the in-text link.
- (void)openLinkWithURL:(const GURL&)URL;

// BVC is completely hidden, detach it from view.
// BVC is completely hidden, detach it from view (for thumbstrip mode).
- (void)dismissBVC;

// Asks the delegate to open history modal with results filtered by
Expand All @@ -80,6 +80,9 @@ enum class TabGridPageConfiguration {
// Asks the delegate to open a new tab page with a web search for |searchText|.
- (void)openSearchResultsPageForSearchText:(NSString*)searchText;

// Sets BVC accessibilityViewIsModal to |modal| (for thumbstrip mode).
- (void)setBVCAccessibilityViewModal:(BOOL)modal;

@end

// View controller representing a tab switcher. The tab switcher has an
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -860,19 +860,34 @@ - (void)didAnimateViewRevealFromState:(ViewRevealState)startViewRevealState
trigger:(ViewRevealTrigger)trigger {
[self updateNotSelectedTabCellOpacityForState:currentViewRevealState];
self.currentState = currentViewRevealState;
// Update a11y visibility for browser and grid. Both should be visible
// when in Peeked mode, and only one visible in the other two modes.
BOOL updateAccessibility = NO;
switch (currentViewRevealState) {
case ViewRevealState::Hidden:
[self.delegate tabGridViewControllerDidDismiss:self];
self.view.accessibilityViewIsModal = NO;
[self.delegate setBVCAccessibilityViewModal:YES];
updateAccessibility = YES;
break;
case ViewRevealState::Peeked:
// No-op.
self.view.accessibilityViewIsModal = NO;
[self.delegate setBVCAccessibilityViewModal:NO];
updateAccessibility = startViewRevealState == ViewRevealState::Hidden;
break;
case ViewRevealState::Revealed:
self.scrollView.scrollEnabled = YES;
[self setInsetForRemoteTabs];
[self.delegate dismissBVC];
self.view.accessibilityViewIsModal = YES;
[self.delegate setBVCAccessibilityViewModal:NO];
updateAccessibility = startViewRevealState == ViewRevealState::Hidden;
break;
}
if (updateAccessibility) {
UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification,
nil);
}
}

// Sets the expected opacity level for each view revealing state.
Expand Down

0 comments on commit ac2fb51

Please sign in to comment.