Simon Hong
c84bd6d27d
Resolves https://github.com/brave/brave-browser/issues/54449 This crash happens when tab search button the only pinned button. I can repro this crash with Chrome(147.0.7727.56) also when search button is pinned. (edited) In chrome, tab search button position varies per platforms. If tab search button is at tab strip(not toolbar), this crash doesn't happen. In Brave, HasTabSearchToolbarButton() is always true, so PinnedToolbarActionsModel migration adds kActionTabSearch to pinned_buttons_. However, browser_actions.cc initializes kActionTabSearch as kNotPinnable, causing ToolbarController::GetDefaultResponsiveElements() to exclude it from responsive_elements_ when the toolbar is first built. When the toolbar is narrow enough that the overflow button appears (because kActionTabSearch is in pinned_buttons_), clicking the overflow button crashes because CreateOverflowMenuModel() iterates responsive_elements_ — where kActionTabSearch is absent — producing an empty menu model with no submenu. Fix by overriding kActionTabSearch to kPinnable in BraveBrowserActions::InitializeBrowserActions(), which runs before toolbar_->Init(). This ensures GetDefaultResponsiveElements() includes kActionTabSearch in responsive_elements_ so the overflow menu populates correctly. Manual test: 1. Launch Brave 2. Make search tab button is the only pinned button in toolbar 3. Resize browser window till overflow button is visible 4. Click overflow button Chromium changes: https://chromium.googlesource.com/chromium/src/+/e80c78d2936c935b0f1c951c0ccf341b4fd3dbf8 commit e80c78d2936c935b0f1c951c0ccf341b4fd3dbf8 Author: Eshwar Stalin <estalin@chromium.org> Date: Wed Feb 18 22:12:10 2026 -0800 [Vertical Tabs] Fix an issue where tab search toolbar button is visible incorrectly This was a regression that was introduced with a previous change. This was caused because the initial state at startup was incorrect. To fix this updating the default pinning state to not pinnable and instead updating that through the tab search toolbar controller. This ensures the correct behavior. Fixed: 485672653 Change-Id: I317e3f540f4db82cb765bd15621b33c4b964badb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7591063 Commit-Queue: Eshwar Stalin <estalin@chromium.org> Reviewed-by: Tom Lukaszewicz <tluk@chromium.org> Cr-Commit-Position: refs/heads/main@{#1586899}
37 lines
1.7 KiB
C++
37 lines
1.7 KiB
C++
/* Copyright (c) 2026 The Brave Authors. All rights reserved.
|
|
* This Source Code Form is subject to the terms of the Mozilla Public
|
|
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
|
|
* You can obtain one at https://mozilla.org/MPL/2.0/. */
|
|
|
|
#include "chrome/browser/ui/actions/chrome_action_id.h"
|
|
#include "chrome/browser/ui/tab_search_feature.h"
|
|
#include "chrome/test/base/in_process_browser_test.h"
|
|
#include "content/public/test/browser_test.h"
|
|
#include "ui/actions/actions.h"
|
|
|
|
using BraveBrowserActionsBrowserTest = InProcessBrowserTest;
|
|
|
|
// Regression test for https://github.com/brave/brave-browser/issues/54449
|
|
//
|
|
// kActionTabSearch must be kPinnable immediately after InitializeBrowserActions
|
|
// so that ToolbarController::GetDefaultResponsiveElements() includes it in
|
|
// responsive_elements_. Without this, the overflow menu is empty when
|
|
// kActionTabSearch overflows, causing a crash on click.
|
|
IN_PROC_BROWSER_TEST_F(BraveBrowserActionsBrowserTest,
|
|
TabSearchActionIsPinnableAfterInit) {
|
|
ASSERT_TRUE(features::HasTabSearchToolbarButton())
|
|
<< "Tab search toolbar button not enabled";
|
|
|
|
auto& action_manager = actions::ActionManager::GetForTesting();
|
|
auto* tab_search_action = action_manager.FindAction(kActionTabSearch);
|
|
ASSERT_NE(tab_search_action, nullptr)
|
|
<< "kActionTabSearch not found in ActionManager";
|
|
|
|
const actions::ActionPinnableState pinnable_state =
|
|
static_cast<actions::ActionPinnableState>(
|
|
tab_search_action->GetProperty(actions::kActionItemPinnableKey));
|
|
EXPECT_EQ(pinnable_state, actions::ActionPinnableState::kPinnable)
|
|
<< "kActionTabSearch must be kPinnable so ToolbarController includes it "
|
|
"in responsive_elements_";
|
|
}
|