From fbdac200d8c42fd5089b16cbcec9f808f058e00a Mon Sep 17 00:00:00 2001 From: Sangwoo Ko Date: Fri, 28 Nov 2025 17:55:19 +0900 Subject: [PATCH] Add flag to force popups to open as tabs (#32380) * Add flag to force popups to open as tabs As per user request, the following changes overrides WindowOpenDisposition::NEW_POPUP to WindowOpenDisposition::NEW_FOREGROUND_TAB when the feature flag kForcePopupToBeOpenedAsTab is enabled. --- browser/about_flags.cc | 10 +++ browser/ui/brave_ui_features.cc | 4 + browser/ui/brave_ui_features.h | 2 + browser/ui/browser_navigator_browsertest.cc | 79 +++++++++++++++++++ .../chrome/browser/ui/browser_navigator.cc | 15 +++- test/BUILD.gn | 2 + 6 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 browser/ui/browser_navigator_browsertest.cc diff --git a/browser/about_flags.cc b/browser/about_flags.cc index 838804cc556..f7362b904ab 100644 --- a/browser/about_flags.cc +++ b/browser/about_flags.cc @@ -716,6 +716,15 @@ constexpr flags_ui::FeatureEntry::Choice kVerticalTabCollapseDelayChoices[] = { FEATURE_VALUE_TYPE(features::kBraveWebAssemblyJitless), \ })) +#define BRAVE_FORCE_POPUP_TO_BE_OPENED_IN_NEW_TAB_FEATURE_ENTRY \ + EXPAND_FEATURE_ENTRIES({ \ + "force-popup-to-be-opened-as-tab", \ + "Force popups to be opened as tab", \ + "Forces all popup windows to be opened in a new tab instead.", \ + kOsDesktop, \ + FEATURE_VALUE_TYPE(features::kForcePopupToBeOpenedAsTab), \ + }) + // Keep the last item empty. #define LAST_BRAVE_FEATURE_ENTRIES_ITEM @@ -1289,6 +1298,7 @@ constexpr flags_ui::FeatureEntry::Choice kVerticalTabCollapseDelayChoices[] = { BRAVE_EDUCATION_FEATURE_ENTRIES \ BRAVE_UPDATER_FEATURE_ENTRIES \ PSST_FEATURE_ENTRIES \ + BRAVE_FORCE_POPUP_TO_BE_OPENED_IN_NEW_TAB_FEATURE_ENTRY \ LAST_BRAVE_FEATURE_ENTRIES_ITEM // Keep it as the last item. namespace flags_ui { namespace { diff --git a/browser/ui/brave_ui_features.cc b/browser/ui/brave_ui_features.cc index 01a05c4450a..66bf15ebe8a 100644 --- a/browser/ui/brave_ui_features.cc +++ b/browser/ui/brave_ui_features.cc @@ -18,4 +18,8 @@ BASE_FEATURE(kBraveWorkaroundNewWindowFlash, base::FEATURE_ENABLED_BY_DEFAULT); #endif // BUILDFLAG(IS_WIN) +// A feature flag to force all popup windows to be opened as tabs. +// https://github.com/brave/brave-browser/issues/40959 +BASE_FEATURE(kForcePopupToBeOpenedAsTab, base::FEATURE_DISABLED_BY_DEFAULT); + } // namespace features diff --git a/browser/ui/brave_ui_features.h b/browser/ui/brave_ui_features.h index dc4a2eecbca..b8989925d2f 100644 --- a/browser/ui/brave_ui_features.h +++ b/browser/ui/brave_ui_features.h @@ -17,6 +17,8 @@ BASE_DECLARE_FEATURE(kBraveFilledBookmarkFolderIcon); BASE_DECLARE_FEATURE(kBraveWorkaroundNewWindowFlash); #endif // BUILDFLAG(IS_WIN) +BASE_DECLARE_FEATURE(kForcePopupToBeOpenedAsTab); + } // namespace features #endif // BRAVE_BROWSER_UI_BRAVE_UI_FEATURES_H_ diff --git a/browser/ui/browser_navigator_browsertest.cc b/browser/ui/browser_navigator_browsertest.cc new file mode 100644 index 00000000000..b260783361a --- /dev/null +++ b/browser/ui/browser_navigator_browsertest.cc @@ -0,0 +1,79 @@ +// Copyright (c) 2025 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 "base/test/run_until.h" +#include "base/test/scoped_feature_list.h" +#include "brave/browser/ui/brave_ui_features.h" +#include "chrome/browser/ui/browser_list.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "components/blocked_content/popup_blocker_tab_helper.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/browser_test_utils.h" +#include "testing/gtest/include/gtest/gtest.h" + +class BrowserNavigatorPopupAsTabBrowserTest + : public InProcessBrowserTest, + public ::testing::WithParamInterface { + public: + BrowserNavigatorPopupAsTabBrowserTest() { + EXPECT_FALSE( + base::FeatureList::IsEnabled(features::kForcePopupToBeOpenedAsTab)) + << "Feature should be disabled by default"; + if (GetParam()) { + feature_list_.InitAndEnableFeature(features::kForcePopupToBeOpenedAsTab); + } else { + feature_list_.InitAndDisableFeature(features::kForcePopupToBeOpenedAsTab); + } + } + ~BrowserNavigatorPopupAsTabBrowserTest() override = default; + + bool ShouldOpenPopupAsTab() const { + return base::FeatureList::IsEnabled(features::kForcePopupToBeOpenedAsTab); + } + + private: + base::test::ScopedFeatureList feature_list_; +}; + +IN_PROC_BROWSER_TEST_P(BrowserNavigatorPopupAsTabBrowserTest, OpenPopupAsTab) { + ASSERT_EQ(1, browser()->tab_strip_model()->count()); + auto* web_contents = browser()->GetTabStripModel()->GetWebContentsAt(0); + ASSERT_TRUE(content::ExecJs( + web_contents->GetPrimaryMainFrame(), + "window.open('about:blank', '_blank', 'height=200,width=150');")); + auto* popup_blocker_tab_helper = + blocked_content::PopupBlockerTabHelper::FromWebContents(web_contents); + ASSERT_TRUE(popup_blocker_tab_helper); + + if (ShouldOpenPopupAsTab()) { + EXPECT_TRUE(base::test::RunUntil([&]() { + if (popup_blocker_tab_helper->GetBlockedPopupsCount() != 0) { + popup_blocker_tab_helper->ShowAllBlockedPopups(); + } + return browser()->tab_strip_model()->count() == 2; + })); + EXPECT_EQ(1u, BrowserList::GetInstance()->size()); + } else { + EXPECT_TRUE(base::test::RunUntil([&]() { + if (popup_blocker_tab_helper->GetBlockedPopupsCount() != 0) { + popup_blocker_tab_helper->ShowAllBlockedPopups(); + } + return BrowserList::GetInstance()->size() == 2; + })); + for (auto& b : *BrowserList::GetInstance()) { + if (b == browser()) { + continue; + } + + EXPECT_TRUE(b->is_type_popup()); + } + EXPECT_EQ(1, browser()->tab_strip_model()->count()); + } +} + +INSTANTIATE_TEST_SUITE_P(All, + BrowserNavigatorPopupAsTabBrowserTest, + ::testing::Bool()); diff --git a/chromium_src/chrome/browser/ui/browser_navigator.cc b/chromium_src/chrome/browser/ui/browser_navigator.cc index 4a9195f3349..cf8b4c3b12a 100644 --- a/chromium_src/chrome/browser/ui/browser_navigator.cc +++ b/chromium_src/chrome/browser/ui/browser_navigator.cc @@ -5,6 +5,7 @@ #include +#include "brave/browser/ui/brave_ui_features.h" #include "brave/components/containers/buildflags/buildflags.h" #include "chrome/browser/tab_contents/tab_util.h" #include "chrome/browser/ui/browser_navigator_params.h" @@ -21,9 +22,21 @@ void UpdateBraveScheme(NavigateParams* params) { } } +void MaybeOverridePopupDisposition(NavigateParams* params) { + if (base::FeatureList::IsEnabled(features::kForcePopupToBeOpenedAsTab) && + params->disposition == WindowOpenDisposition::NEW_POPUP) { + params->disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB; + } +} + +void UpdateParams(NavigateParams* params) { + UpdateBraveScheme(params); + MaybeOverridePopupDisposition(params); +} + } // namespace -#define BRAVE_ADJUST_NAVIGATE_PARAMS_FOR_URL UpdateBraveScheme(params); +#define BRAVE_ADJUST_NAVIGATE_PARAMS_FOR_URL UpdateParams(params); #if BUILDFLAG(ENABLE_CONTAINERS) #define GetSiteInstanceForNewTab(...) \ diff --git a/test/BUILD.gn b/test/BUILD.gn index 6f724b6d2df..23aa61a3a90 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1178,6 +1178,7 @@ test("brave_browser_tests") { "//brave/browser/brave_shields/ad_block_custom_resources_browsertest.cc", "//brave/browser/brave_shields/ad_block_only_mode_browsertest.cc", "//brave/browser/ssl/certificate_transparency_browsertest.cc", + "//brave/browser/ui/browser_navigator_browsertest.cc", "//brave/browser/ui/toolbar/brave_location_bar_model_delegate_browsertest.cc", "//brave/browser/ui/views/toolbar/wallet_button_notification_source_browsertest.cc", ] @@ -1206,6 +1207,7 @@ test("brave_browser_tests") { "//chrome/browser/ui/webui/side_panel/bookmarks:mojo_bindings", "//components/autofill/content/browser", "//components/autofill/core/browser", + "//components/blocked_content", ] }