Commit e9a400c4 authored by Abigail Klein's avatar Abigail Klein Committed by Chromium LUCI CQ

[Live Caption] Keep the caption bubble inactive until it is explicitly

activated by the focus manager.

Bug: 1055150, 1156235
Change-Id: Iafc65c8f3f4ba0a9fa57a9db11d1c1a9cd5710fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582298
Commit-Queue: Abigail Klein <abigailbklein@google.com>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835690}
parent d8c35f21
...@@ -686,19 +686,11 @@ void CaptionBubble::OnWidgetVisibilityChanged(views::Widget* widget, ...@@ -686,19 +686,11 @@ void CaptionBubble::OnWidgetVisibilityChanged(views::Widget* widget,
// The caption bubble can only be activated when it is visible. Nothing else, // The caption bubble can only be activated when it is visible. Nothing else,
// including the focus manager, can activate the caption bubble. // including the focus manager, can activate the caption bubble.
SetCanActivate(visible); SetCanActivate(visible);
// Ensure that the widget is only activated when it is visible. // Ensure that the widget is deactivated when it is hidden.
// TODO(crbug.com/1144201): Investigate whether Hide() should always // TODO(crbug.com/1144201): Investigate whether Hide() should always
// deactivate widgets, and if so, remove this. // deactivate widgets, and if so, remove this.
if (visible) { if (!visible)
#if !defined(OS_MAC)
// On MacOS browsertests, which do not have an activation policy, the widget
// might already be activated.
DCHECK(!widget->IsActive());
#endif
widget->Activate();
} else {
widget->Deactivate(); widget->Deactivate();
}
} }
void CaptionBubble::UpdateCaptionStyle( void CaptionBubble::UpdateCaptionStyle(
......
...@@ -450,6 +450,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, ...@@ -450,6 +450,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
"Honeybees have tiny hairs on their eyes to help them collect pollen"); "Honeybees have tiny hairs on their eyes to help them collect pollen");
// Not focused initially. // Not focused initially.
EXPECT_FALSE(GetBubble()->HasFocus()); EXPECT_FALSE(GetBubble()->HasFocus());
// In the tests, the widget must be active for the key presses to be handled.
GetCaptionWidget()->Activate();
// Key presses do not change the bounds when it is not focused. // Key presses do not change the bounds when it is not focused.
gfx::Rect bounds = GetCaptionWidget()->GetClientAreaBoundsInScreen(); gfx::Rect bounds = GetCaptionWidget()->GetClientAreaBoundsInScreen();
...@@ -503,6 +505,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) { ...@@ -503,6 +505,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) {
EXPECT_FALSE(GetBubble()->HasFocus()); EXPECT_FALSE(GetBubble()->HasFocus());
EXPECT_FALSE(GetCloseButton()->HasFocus()); EXPECT_FALSE(GetCloseButton()->HasFocus());
EXPECT_FALSE(GetBubble()->GetFocusManager()->GetFocusedView()); EXPECT_FALSE(GetBubble()->GetFocusManager()->GetFocusedView());
// In the tests, the widget must be active for the key presses to be handled.
GetCaptionWidget()->Activate();
// Press tab until we enter the bubble. // Press tab until we enter the bubble.
while (!GetBubble()->HasFocus()) { while (!GetBubble()->HasFocus()) {
...@@ -1043,6 +1047,10 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, ...@@ -1043,6 +1047,10 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
OnPartialTranscription("Cows can detect odors up to 6 miles away."); OnPartialTranscription("Cows can detect odors up to 6 miles away.");
EXPECT_TRUE(IsWidgetVisible()); EXPECT_TRUE(IsWidgetVisible());
EXPECT_TRUE(CanWidgetActivate()); EXPECT_TRUE(CanWidgetActivate());
EXPECT_FALSE(IsWidgetActive());
GetBubble()->RequestFocus();
EXPECT_TRUE(IsWidgetVisible());
EXPECT_TRUE(CanWidgetActivate());
EXPECT_TRUE(IsWidgetActive()); EXPECT_TRUE(IsWidgetActive());
ClickButton(GetCloseButton()); ClickButton(GetCloseButton());
EXPECT_FALSE(IsWidgetVisible()); EXPECT_FALSE(IsWidgetVisible());
...@@ -1081,6 +1089,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, HidesAfterInactivity) { ...@@ -1081,6 +1089,8 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, HidesAfterInactivity) {
test_task_runner->FastForwardBy(base::TimeDelta::FromSeconds(4)); test_task_runner->FastForwardBy(base::TimeDelta::FromSeconds(4));
EXPECT_TRUE(IsWidgetVisible()); EXPECT_TRUE(IsWidgetVisible());
// In the tests, the widget must be active.
GetCaptionWidget()->Activate();
// Caption bubble stays visible while it has focus. // Caption bubble stays visible while it has focus.
GetBubble()->RequestFocus(); GetBubble()->RequestFocus();
EXPECT_TRUE(IsWidgetVisible()); EXPECT_TRUE(IsWidgetVisible());
......
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