Commit bc22eed4 authored by Katie D's avatar Katie D Committed by Commit Bot

Rotating focus with F6 includes the caption bubble.

Allows users to focus into and out of the caption bubble using F6,
which rotates focus between focusable panes.

AX-Relnotes: N/A
Bug: 1055150
Change-Id: I82efd54644fbc1cada2cb3e2f5cf4c52da21336a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2197255Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarAbigail Klein <abigailbklein@google.com>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770294}
parent c414e156
......@@ -167,14 +167,19 @@ void CaptionController::DispatchTranscription(
content::WebContents* web_contents,
const chrome::mojom::TranscriptionResultPtr& transcription_result) {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
if (!browser)
return;
if (!caption_bubble_controllers_.count(browser))
if (!browser || !caption_bubble_controllers_.count(browser))
return;
caption_bubble_controllers_[browser]->OnTranscription(transcription_result,
web_contents);
}
CaptionBubbleController*
CaptionController::GetCaptionBubbleControllerForBrowser(Browser* browser) {
if (!browser || !caption_bubble_controllers_.count(browser))
return nullptr;
return caption_bubble_controllers_[browser].get();
}
void CaptionController::UpdateCaptionStyle() {
PrefService* profile_prefs = profile_->GetPrefs();
// Metrics are recorded when passing the caption prefs to the browser, so do
......
......@@ -73,6 +73,9 @@ class CaptionController : public BrowserListObserver, public KeyedService {
content::WebContents* web_contents,
const chrome::mojom::TranscriptionResultPtr& transcription_result);
CaptionBubbleController* GetCaptionBubbleControllerForBrowser(
Browser* browser);
private:
friend class CaptionControllerFactory;
......
......@@ -14,6 +14,7 @@
#include "base/strings/string16.h"
#include "build/build_config.h"
#include "chrome/browser/accessibility/caption_controller.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/grit/generated_resources.h"
#include "components/vector_icons/vector_icons.h"
#include "third_party/re2/src/re2/re2.h"
......@@ -154,16 +155,20 @@ class CaptionBubbleFrameView : public views::BubbleFrameView {
};
CaptionBubble::CaptionBubble(views::View* anchor,
BrowserView* browser_view,
base::OnceClosure destroyed_callback)
: BubbleDialogDelegateView(anchor,
views::BubbleBorder::FLOAT,
views::BubbleBorder::Shadow::NO_SHADOW),
destroyed_callback_(std::move(destroyed_callback)),
ratio_in_parent_x_(kDefaultRatioInParentX),
ratio_in_parent_y_(kDefaultRatioInParentY) {
ratio_in_parent_y_(kDefaultRatioInParentY),
browser_view_(browser_view) {
SetButtons(ui::DIALOG_BUTTON_NONE);
set_draggable(true);
AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
AddAccelerator(ui::Accelerator(ui::VKEY_F6, ui::EF_NONE));
AddAccelerator(ui::Accelerator(ui::VKEY_F6, ui::EF_SHIFT_DOWN));
// The CaptionBubble is focusable. It will alert the CaptionBubbleFrameView
// when its focus changes so that the focus ring can be updated.
// TODO(crbug.com/1055150): Consider using
......@@ -378,14 +383,25 @@ void CaptionBubble::OnKeyEvent(ui::KeyEvent* event) {
}
bool CaptionBubble::AcceleratorPressed(const ui::Accelerator& accelerator) {
DCHECK_EQ(accelerator.key_code(), ui::VKEY_ESCAPE);
// We don't want to close when the user hits "escape", because this isn't a
// normal dialog bubble -- it's meant to be up all the time. We just want to
// release focus back to the page in that case.
// Users should use the "close" button to close the bubble.
// TODO(crbug.com/1055150): This doesn't work in Mac.
GetAnchorView()->RequestFocus();
return true;
if (accelerator.key_code() == ui::VKEY_ESCAPE) {
// We don't want to close when the user hits "escape", because this isn't a
// normal dialog bubble -- it's meant to be up all the time. We just want to
// release focus back to the page in that case.
// Users should use the "close" button to close the bubble.
GetAnchorView()->RequestFocus();
GetAnchorView()->GetWidget()->Activate();
return true;
}
if (accelerator.key_code() == ui::VKEY_F6) {
// F6 rotates focus through the panes in the browser. Use
// BrowserView::AcceleratorPressed so that metrics are logged appropriately.
browser_view_->AcceleratorPressed(accelerator);
// Remove focus from this widget.
browser_view_->GetWidget()->Activate();
return true;
}
NOTREACHED();
return false;
}
void CaptionBubble::OnFocus() {
......@@ -498,6 +514,10 @@ void CaptionBubble::Hide() {
UpdateBubbleVisibility();
}
const char* CaptionBubble::GetClassName() const {
return "CaptionBubble";
}
double CaptionBubble::GetTextScaleFactor() {
double textScaleFactor = 1;
if (caption_style_) {
......
......@@ -21,6 +21,8 @@ namespace ui {
struct AXNodeData;
}
class BrowserView;
namespace captions {
class CaptionBubbleFrameView;
......@@ -33,7 +35,9 @@ class CaptionBubbleFrameView;
class CaptionBubble : public views::BubbleDialogDelegateView,
public views::ButtonListener {
public:
CaptionBubble(views::View* anchor, base::OnceClosure destroyed_callback);
CaptionBubble(views::View* anchor,
BrowserView* browser_view,
base::OnceClosure destroyed_callback);
~CaptionBubble() override;
CaptionBubble(const CaptionBubble&) = delete;
CaptionBubble& operator=(const CaptionBubble&) = delete;
......@@ -56,6 +60,8 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
// directly.
void Hide();
const char* GetClassName() const override;
protected:
// views::BubbleDialogDelegateView:
void Init() override;
......@@ -113,6 +119,9 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
// Whether we should show the widget. False if explicitly asked to hide.
bool should_show_ = true;
// A reference to the BrowserView holding this bubble. Unowned.
BrowserView* browser_view_;
};
} // namespace captions
......
......@@ -6,6 +6,8 @@
#include <memory>
#include "chrome/browser/accessibility/caption_controller.h"
#include "chrome/browser/accessibility/caption_controller_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/views/accessibility/caption_bubble.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
......@@ -19,11 +21,26 @@ std::unique_ptr<CaptionBubbleController> CaptionBubbleController::Create(
return std::make_unique<CaptionBubbleControllerViews>(browser);
}
// Static
views::View* CaptionBubbleControllerViews::GetCaptionBubbleAccessiblePane(
Browser* browser) {
CaptionController* caption_controller =
CaptionControllerFactory::GetForProfileIfExists(browser->profile());
if (caption_controller) {
CaptionBubbleControllerViews* bubble_controller =
static_cast<CaptionBubbleControllerViews*>(
caption_controller->GetCaptionBubbleControllerForBrowser(browser));
if (bubble_controller)
return bubble_controller->GetFocusableCaptionBubble();
}
return nullptr;
}
CaptionBubbleControllerViews::CaptionBubbleControllerViews(Browser* browser)
: CaptionBubbleController(browser), browser_(browser) {
BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
caption_bubble_ = new CaptionBubble(
browser_view->GetContentsView(),
browser_view->GetContentsView(), browser_view,
base::BindOnce(&CaptionBubbleControllerViews::OnCaptionBubbleDestroyed,
base::Unretained(this)));
caption_widget_ =
......@@ -101,4 +118,10 @@ void CaptionBubbleControllerViews::UpdateCaptionStyle(
caption_bubble_->UpdateCaptionStyle(caption_style);
}
views::View* CaptionBubbleControllerViews::GetFocusableCaptionBubble() {
if (caption_widget_ && caption_widget_->IsVisible())
return caption_bubble_;
return nullptr;
}
} // namespace captions
......@@ -12,6 +12,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
namespace views {
class View;
class Widget;
}
......@@ -36,6 +37,8 @@ struct CaptionText {
class CaptionBubbleControllerViews : public CaptionBubbleController,
public TabStripModelObserver {
public:
static views::View* GetCaptionBubbleAccessiblePane(Browser* browser);
explicit CaptionBubbleControllerViews(Browser* browser);
~CaptionBubbleControllerViews() override;
CaptionBubbleControllerViews(const CaptionBubbleControllerViews&) = delete;
......@@ -51,6 +54,10 @@ class CaptionBubbleControllerViews : public CaptionBubbleController,
void UpdateCaptionStyle(
base::Optional<ui::CaptionStyle> caption_style) override;
// Returns the view of the caption bubble which should receive focus, if one
// exists.
views::View* GetFocusableCaptionBubble();
private:
friend class CaptionBubbleControllerViewsTest;
......@@ -81,7 +88,6 @@ class CaptionBubbleControllerViews : public CaptionBubbleController,
// transcription in a while.
std::unordered_map<content::WebContents*, CaptionText> caption_texts_;
};
} // namespace captions
#endif // CHROME_BROWSER_UI_VIEWS_ACCESSIBILITY_CAPTION_BUBBLE_CONTROLLER_VIEWS_H_
......@@ -392,8 +392,6 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
false, false, false));
EXPECT_EQ(bounds, GetCaptionWidget()->GetClientAreaBoundsInScreen());
#if !defined(OS_MACOSX)
// TODO(crbug.com/1055150): Get this working for Mac.
// Hitting the escape key should remove focus from the view, so arrows no
// longer work.
EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_ESCAPE, false,
......@@ -402,7 +400,6 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest,
EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_UP, false,
false, false, false));
EXPECT_EQ(bounds, GetCaptionWidget()->GetClientAreaBoundsInScreen());
#endif
}
IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) {
......@@ -415,9 +412,10 @@ IN_PROC_BROWSER_TEST_F(CaptionBubbleControllerViewsTest, FocusableInTabOrder) {
EXPECT_FALSE(GetBubble()->GetFocusManager()->GetFocusedView());
// Press tab until we enter the bubble.
while (!GetBubble()->HasFocus())
while (!GetBubble()->HasFocus()) {
EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_TAB, false,
false, false, false));
}
#if defined(USE_AURA) && !defined(OS_CHROMEOS)
// Check the native widget has focus.
aura::client::FocusClient* focus_client =
......
......@@ -75,6 +75,7 @@
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/accelerator_table.h"
#include "chrome/browser/ui/views/accessibility/accessibility_focus_highlight.h"
#include "chrome/browser/ui/views/accessibility/caption_bubble_controller_views.h"
#include "chrome/browser/ui/views/accessibility/invert_bubble_view.h"
#include "chrome/browser/ui/views/autofill/autofill_bubble_handler_impl.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
......@@ -2558,6 +2559,12 @@ void BrowserView::GetAccessiblePanes(std::vector<views::View*>* panes) {
panes->push_back(infobar_container_);
if (download_shelf_.get())
panes->push_back(download_shelf_.get());
// See if there is a caption bubble present.
views::View* caption_bubble =
captions::CaptionBubbleControllerViews::GetCaptionBubbleAccessiblePane(
browser());
if (caption_bubble)
panes->push_back(caption_bubble);
panes->push_back(contents_web_view_);
if (devtools_web_view_->GetVisible())
panes->push_back(devtools_web_view_);
......
......@@ -7,15 +7,19 @@
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "chrome/browser/accessibility/caption_controller.h"
#include "chrome/browser/accessibility/caption_controller_factory.h"
#include "chrome/browser/devtools/devtools_window_testing.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/tab_modal_confirm_dialog.h"
#include "chrome/browser/ui/tab_ui_helper.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/views/accessibility/caption_bubble_controller_views.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view_observer.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/test/base/in_process_browser_test.h"
......@@ -28,13 +32,21 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "media/base/media_switches.h"
#include "ui/accessibility/platform/ax_platform_node.h"
#include "ui/accessibility/platform/ax_platform_node_test_helper.h"
#include "ui/base/l10n/l10n_util.h"
#if defined(USE_AURA)
#include "ui/aura/client/focus_client.h"
#include "ui/views/widget/native_widget_aura.h"
#endif // USE_AURA
class BrowserViewTest : public InProcessBrowserTest {
public:
BrowserViewTest() : InProcessBrowserTest(), devtools_(nullptr) {}
BrowserViewTest() : devtools_(nullptr) {
scoped_feature_list_.InitAndEnableFeature(media::kLiveCaption);
}
protected:
BrowserView* browser_view() {
......@@ -65,6 +77,8 @@ class BrowserViewTest : public InProcessBrowserTest {
DevToolsWindow* devtools_;
private:
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(BrowserViewTest);
};
......@@ -364,3 +378,55 @@ IN_PROC_BROWSER_TEST_F(BrowserViewTest, GetAccessibleTabModalDialogTree) {
}
#endif // !defined(OS_MACOSX)
IN_PROC_BROWSER_TEST_F(BrowserViewTest, F6CyclesThroughCaptionBubbleToo) {
captions::CaptionController* caption_controller =
captions::CaptionControllerFactory::GetForProfileIfExists(
browser()->profile());
caption_controller->Init();
browser()->profile()->GetPrefs()->SetBoolean(prefs::kLiveCaptionEnabled,
true);
// No bubble is shown until a transcription happens.
captions::CaptionBubbleControllerViews* bubble_controller =
static_cast<captions::CaptionBubbleControllerViews*>(
caption_controller->GetCaptionBubbleControllerForBrowser(browser()));
EXPECT_FALSE(bubble_controller->GetFocusableCaptionBubble());
caption_controller->DispatchTranscription(
browser()->tab_strip_model()->GetActiveWebContents(),
chrome::mojom::TranscriptionResult::New("Hello, world", false));
// Now the caption bubble exists but is not focused.
views::View* bubble = bubble_controller->GetFocusableCaptionBubble();
EXPECT_TRUE(bubble);
EXPECT_TRUE(bubble->GetWidget()->IsVisible());
EXPECT_FALSE(bubble->HasFocus());
EXPECT_FALSE(bubble->GetFocusManager()->GetFocusedView());
// Press F6 until we enter the bubble.
while (!bubble->HasFocus()) {
EXPECT_TRUE(
browser_view()->AcceleratorPressed(ui::Accelerator(ui::VKEY_F6, 0)));
}
#if defined(USE_AURA) && !defined(OS_CHROMEOS)
// Check the native widget has focus.
aura::client::FocusClient* focus_client =
aura::client::GetFocusClient(bubble->GetWidget()->GetNativeView());
EXPECT_TRUE(bubble->GetWidget()->GetNativeView() ==
focus_client->GetFocusedWindow());
#endif
// F6 again exits the bubble. Because the bubble is focused, it gets the
// accelerator event.
EXPECT_TRUE(bubble->AcceleratorPressed(ui::Accelerator(ui::VKEY_F6, 0)));
// Now something else within the browser_view's focus manager is focused.
EXPECT_FALSE(bubble->HasFocus());
EXPECT_FALSE(bubble->GetFocusManager()->GetFocusedView());
EXPECT_TRUE(browser_view()->GetWidget()->GetFocusManager()->GetFocusedView());
#if defined(USE_AURA) && !defined(OS_CHROMEOS)
// The bubble's native widget should no longer have focus.
EXPECT_FALSE(bubble->GetWidget()->GetNativeView() ==
focus_client->GetFocusedWindow());
#endif
}
\ No newline at end of file
......@@ -206,7 +206,9 @@ bool FocusManager::RotatePaneFocus(Direction direction,
continue;
pane->RequestFocus();
focused_view = GetFocusedView();
// |pane| may be in a different widget, so don't assume its focus manager
// is |this|.
focused_view = pane->GetWidget()->GetFocusManager()->GetFocusedView();
if (pane == focused_view || pane->Contains(focused_view))
return true;
}
......
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