Commit 23213dd6 authored by Keren Zhu's avatar Keren Zhu Committed by Commit Bot

Don't clear kAnchoredDialogKey when an non-focusable widget dismisses

An non-focasable widget will not set its anchor view's kAnchoredDialogKey,
but currently a bubble dialog will reset kAnchoredDialogKey when it
detaches from its anchor view. This will break the focus traversal path
if kAnchoredDialogKey was pointing to a different widget.

This CL fixes this issue by resetting kAnchoredDialogKey only if it
is pointing to the current bubble. This issue blocks bug 1121399
where a focusable IPH bubble will contest kAnchoredDialogKey of a tab
while a non-focusable tab hover card might anchor to it. The hover card
will later accidentally detaches the IPH bubble from the tab when it
dismisses.

Bug: 1121399
Change-Id: Ia7f82ee166eae100cd18ffa816ddf50cf02acba6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422709Reviewed-by: default avatarWei Li <weili@chromium.org>
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809433}
parent 7aa6ce70
......@@ -447,7 +447,12 @@ void BubbleDialogDelegate::OnBubbleWidgetClosing() {
// avoids a bug that occured when a focused anchor view is made unfocusable
// right after the bubble is closed. Previously, focus would advance into the
// bubble then would be lost when the bubble was destroyed.
if (GetAnchorView())
//
// If kAnchoredDialogKey does not point to |this|, then |this| is not on the
// focus traversal path. Don't reset kAnchoredDialogKey or we risk detaching
// a widget from the traversal path.
if (GetAnchorView() &&
GetAnchorView()->GetProperty(kAnchoredDialogKey) == this)
GetAnchorView()->ClearProperty(kAnchoredDialogKey);
}
......@@ -642,7 +647,8 @@ void BubbleDialogDelegate::SetAnchorView(View* anchor_view) {
anchor_widget_observer_.reset();
}
if (GetAnchorView()) {
GetAnchorView()->ClearProperty(kAnchoredDialogKey);
if (GetAnchorView()->GetProperty(kAnchoredDialogKey) == this)
GetAnchorView()->ClearProperty(kAnchoredDialogKey);
anchor_view_observer_.reset();
}
......@@ -685,6 +691,11 @@ void BubbleDialogDelegate::SetAnchorView(View* anchor_view) {
if (anchor_view && focus_traversable_from_anchor_view_) {
// Make sure that focus can move into here from the anchor view (but not
// out, focus will cycle inside the dialog once it gets here).
// It is possible that a view anchors more than one widgets,
// but among them there should be at most one widget that is focusable.
auto* old_anchored_dialog = anchor_view->GetProperty(kAnchoredDialogKey);
if (old_anchored_dialog && old_anchored_dialog != this)
DLOG(WARNING) << "|anchor_view| has already anchored a focusable widget.";
anchor_view->SetProperty(kAnchoredDialogKey, this);
}
}
......
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