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.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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_
|
||||
|
||||
@@ -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<bool> {
|
||||
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());
|
||||
@@ -5,6 +5,7 @@
|
||||
|
||||
#include <string_view>
|
||||
|
||||
#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(...) \
|
||||
|
||||
@@ -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",
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user