Commit 0a0395f5 authored by Mark Cogan's avatar Mark Cogan Committed by Commit Bot

[iOS] Fix VoiceOver on tab grid after closing last incognito tab.

When closing the last incognito tab, the tab grid pauses briefly
before scrolling to the regular tabs page. With VoiceOver enabled,
the text on the Incognito page gets focused before this scrolling
happens, which causes the scroll view to stay on the regular tabs
page. (See the bug for more details).

This CL removes the delay in scrolling when VoiceOver is enabled,
immediately scrolling to the regular tabs page when the last
incognito tab is closed. This prevents the timing issues that cause
the text focus error.

Bug: 980844
Change-Id: Idee32ce9863579372de9383644b3f24d02342960
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899995
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: default avatarRobbie Gibson <rkgibson@google.com>
Auto-Submit: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713495}
parent e5f5c1f8
...@@ -500,7 +500,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) { ...@@ -500,7 +500,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
// to match. If |animated| is YES, the scroll view change may animate; if it is // to match. If |animated| is YES, the scroll view change may animate; if it is
// NO, it will never animate. // NO, it will never animate.
- (void)scrollToPage:(TabGridPage)targetPage animated:(BOOL)animated { - (void)scrollToPage:(TabGridPage)targetPage animated:(BOOL)animated {
// This method should never early return if |currentPage| == |_currentPage|; // This method should never early return if |targetPage| == |_currentPage|;
// the ivar may have been set before the scroll view could be updated. Calling // the ivar may have been set before the scroll view could be updated. Calling
// this method should always update the scroll view's offset if possible. // this method should always update the scroll view's offset if possible.
...@@ -509,21 +509,23 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) { ...@@ -509,21 +509,23 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
self.currentPage = targetPage; self.currentPage = targetPage;
return; return;
} }
CGFloat pageWidth = self.scrollView.frame.size.width; CGFloat pageWidth = self.scrollView.frame.size.width;
NSUInteger pageIndex = GetPageIndexFromPage(targetPage); NSUInteger pageIndex = GetPageIndexFromPage(targetPage);
CGPoint offset = CGPointMake(pageIndex * pageWidth, 0); CGPoint targetOffset = CGPointMake(pageIndex * pageWidth, 0);
// If the view is visible and |animated| is YES, animate the change. // If the view is visible and |animated| is YES, animate the change.
// Otherwise don't. // Otherwise don't.
if (self.view.window == nil || !animated) { if (self.view.window == nil || !animated) {
[self.scrollView setContentOffset:offset animated:NO]; [self.scrollView setContentOffset:targetOffset animated:NO];
self.currentPage = targetPage; self.currentPage = targetPage;
} else { } else {
// Only set |scrollViewAnimatingContentOffset| to YES if there's an actual // Only set |scrollViewAnimatingContentOffset| to YES if there's an actual
// change in the contentOffset, as |-scrollViewDidEndScrollingAnimation:| is // change in the contentOffset, as |-scrollViewDidEndScrollingAnimation:| is
// never called if the animation does not occur. // never called if the animation does not occur.
if (!CGPointEqualToPoint(self.scrollView.contentOffset, offset)) { if (!CGPointEqualToPoint(self.scrollView.contentOffset, targetOffset)) {
self.scrollViewAnimatingContentOffset = YES; self.scrollViewAnimatingContentOffset = YES;
[self.scrollView setContentOffset:offset animated:YES]; [self.scrollView setContentOffset:targetOffset animated:YES];
// |self.currentPage| is set in scrollViewDidEndScrollingAnimation: // |self.currentPage| is set in scrollViewDidEndScrollingAnimation:
} else { } else {
self.currentPage = targetPage; self.currentPage = targetPage;
...@@ -1046,12 +1048,12 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) { ...@@ -1046,12 +1048,12 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
// Sets both the current page and page control's selected page to |page|. // Sets both the current page and page control's selected page to |page|.
// Animation is used if |animated| is YES. // Animation is used if |animated| is YES.
- (void)setCurrentPageAndPageControlSelectedPage:(TabGridPage)page - (void)setCurrentPageAndPageControl:(TabGridPage)page animated:(BOOL)animated {
animated:(BOOL)animated {
if (self.topToolbar.pageControl.selectedPage != page) if (self.topToolbar.pageControl.selectedPage != page)
[self.topToolbar.pageControl setSelectedPage:page animated:animated]; [self.topToolbar.pageControl setSelectedPage:page animated:animated];
if (self.currentPage != page) if (self.currentPage != page) {
[self scrollToPage:page animated:animated]; [self scrollToPage:page animated:animated];
}
} }
#pragma mark - GridViewControllerDelegate #pragma mark - GridViewControllerDelegate
...@@ -1120,20 +1122,26 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) { ...@@ -1120,20 +1122,26 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
if (self.viewLoaded && self.view.window) { if (self.viewLoaded && self.view.window) {
// Visibly scroll to the regular tabs panel after a slight delay when // Visibly scroll to the regular tabs panel after a slight delay when
// the user is already in the tab switcher. // the user is already in the tab switcher.
// Per crbug.com/980844, if the user has VoiceOver enabled, don't delay
// and just animate immediately; delaying the scrolling will cause
// VoiceOver to focus the text on the Incognito page.
__weak TabGridViewController* weakSelf = self; __weak TabGridViewController* weakSelf = self;
base::PostDelayedTask( auto scrollToRegularTabs = ^{
FROM_HERE, {web::WebThread::UI}, base::BindOnce(^{ [weakSelf setCurrentPageAndPageControl:TabGridPageRegularTabs
[weakSelf setCurrentPageAndPageControlSelectedPage: animated:YES];
TabGridPageRegularTabs };
animated:YES]; if (UIAccessibilityIsVoiceOverRunning()) {
}), scrollToRegularTabs();
base::TimeDelta::FromMilliseconds( } else {
kTabGridScrollAnimationDelayInMilliseconds)); base::TimeDelta delay = base::TimeDelta::FromMilliseconds(
kTabGridScrollAnimationDelayInMilliseconds);
base::PostDelayedTask(FROM_HERE, {web::WebThread::UI},
base::BindOnce(scrollToRegularTabs), delay);
}
} else { } else {
// Directly show the regular tabs in tab switcher without animation if // Directly show the regular tab page without animation if
// the user was not already in tab switcher. // the user was not already in tab switcher.
[self setCurrentPageAndPageControlSelectedPage:TabGridPageRegularTabs [self setCurrentPageAndPageControl:TabGridPageRegularTabs animated:NO];
animated:NO];
} }
} }
} }
......
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