Commit f289641e authored by Connie Wan's avatar Connie Wan Committed by Commit Bot

Fix double-show of Tab Group Editor Bubble on Mac

The main fix for the flicker is the check for is_open(), but this caused another issue with the first context menu not showing up. See inline comment for the subsequent fix for the context menu.

Bug: 1068934
Change-Id: Ib1269571b95de5624e0891501e8fb42e77d61611
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144262Reviewed-by: default avatarCharlene Yan <cyan@chromium.org>
Commit-Queue: Connie Wan <connily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757930}
parent f2cb199a
......@@ -152,7 +152,7 @@ bool TabGroupHeader::OnMouseDragged(const ui::MouseEvent& event) {
}
void TabGroupHeader::OnMouseReleased(const ui::MouseEvent& event) {
if (!dragging()) {
if (!dragging() && !editor_bubble_tracker_.is_open()) {
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
tab_strip_->controller()->GetBrowser(), group().value(), this));
}
......@@ -234,9 +234,36 @@ void TabGroupHeader::ShowContextMenuForViewImpl(
if (editor_bubble_tracker_.is_open())
return;
// When the context menu is triggered via keyboard, the keyboard event
// propagates to the textfield inside the Editor Bubble. In those cases, we
// want to tell the Editor Bubble to stop the event by setting
// stop_context_menu_propagation to true.
//
// However, when the context menu is triggered via mouse, the same event
// sequence doesn't happen. Stopping the context menu propagation in that case
// would artificially hide the textfield's context menu the first time the
// user tried to access it. So we don't want to stop the context menu
// propagation if this call is reached via mouse.
//
// Notably, event behavior with a mouse is inconsistent depending on
// OS. When not on Mac, the OnMouseReleased() event happens first and opens
// the Editor Bubble early, preempting the Show() call below. On Mac, the
// ShowContextMenu() event happens first and the Show() call is made here.
//
// So, because of the event order on non-Mac, and because there is no native
// way to open a context menu via keyboard on Mac, we assume that we've
// reached this function via mouse if and only if the current OS is Mac.
// Therefore, we don't stop the menu propagation in that case.
constexpr bool kStopContextMenuPropagation =
#if defined(OS_MACOSX)
false;
#else
true;
#endif
editor_bubble_tracker_.Opened(TabGroupEditorBubbleView::Show(
tab_strip_->controller()->GetBrowser(), group().value(), this,
base::nullopt, nullptr, true));
base::nullopt, nullptr, kStopContextMenuPropagation));
}
int TabGroupHeader::CalculateWidth() const {
......
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