Commit 0e72fa06 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Make sure F6 focus cycle falls on focusable view

Fixes regression after refactoring of DownloadItemView where the new
view implementation is not focusable (but a subset of its children are).

DownloadShelfView returns a DownloadItemView as its first focusable
item (totally reasonable), but since the item itself is not focusable we
need to forward focus to the open button inside it.

I still think it's reasonable to focus an unfocusable item if its
children are focusable, so ideally a fix would change ::SetFocusedView
to focus the first focusable child (if any) or otherwise clear focus.
There's a TODO present inside AccessiblePaneView to address this.

Bug: chromium:1000998
Change-Id: I071d1c59401b35f251cd429ae3b5892462f91adf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1798086Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695678}
parent dd58b7c2
...@@ -77,6 +77,15 @@ bool AccessiblePaneView::SetPaneFocus(views::View* initial_focus) { ...@@ -77,6 +77,15 @@ bool AccessiblePaneView::SetPaneFocus(views::View* initial_focus) {
focus_manager_->SetFocusedView(initial_focus); focus_manager_->SetFocusedView(initial_focus);
// TODO(pbos): Move this behavior into FocusManager. Focusing an unfocusable
// view should do something smart (move focus to its children or clear focus).
// DownloadItemView is an example (isn't focusable, has focusable children).
// See https://crbug.com/1000998.
// The initially-focused view may not be focusable (but one of its children
// might). We may need to advance focus here to make sure focus is on
// something focusable.
focus_manager_->AdvanceFocusIfNecessary();
// If we already have pane focus, we're done. // If we already have pane focus, we're done.
if (pane_has_focus_) if (pane_has_focus_)
return true; 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