From 0f46df7496c8ae780c333cefd5597b96cd6cdbff Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Thu, 22 May 2025 08:09:00 +0900 Subject: [PATCH] Handle contents separator visibility with `SideBySide` (#29126) fix https://github.com/brave/brave-browser/issues/46193 When rounded corners feature is disabled, contents separator should be hidden when split view is opened. TEST=SideBySideEnabledBrowserTest.BraveMultiContentsViewTest --- browser/ui/views/frame/brave_browser_view.cc | 15 ++++++++++++++- browser/ui/views/frame/brave_browser_view.h | 8 ++++++++ .../views/split_view/split_view_browsertest.cc | 17 ++++++++++++++--- .../browser/ui/views/frame/browser_view.h | 4 ++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index 23790bd1823..591e1dc5e8d 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -815,6 +815,18 @@ void BraveBrowserView::GetAccessiblePanes(std::vector* panes) { } } +void BraveBrowserView::ShowSplitView() { + BrowserView::ShowSplitView(); + + UpdateContentsSeparatorVisibility(); +} + +void BraveBrowserView::HideSplitView() { + BrowserView::HideSplitView(); + + UpdateContentsSeparatorVisibility(); +} + bool BraveBrowserView::ShouldShowWindowTitle() const { if (BrowserView::ShouldShowWindowTitle()) { return true; @@ -881,7 +893,8 @@ void BraveBrowserView::UpdateContentsSeparatorVisibility() { // refers it's preferred size. // Don't show that separator as split view has border around contents // container. - if (split_view_ && split_view_->IsSplitViewActive()) { + if ((split_view_ && split_view_->IsSplitViewActive()) || + (multi_contents_view_ && multi_contents_view_->IsInSplitView())) { contents_separator_->SetPreferredSize({}); return; } diff --git a/browser/ui/views/frame/brave_browser_view.h b/browser/ui/views/frame/brave_browser_view.h index 5266317d00a..8b1c0995deb 100644 --- a/browser/ui/views/frame/brave_browser_view.h +++ b/browser/ui/views/frame/brave_browser_view.h @@ -160,6 +160,8 @@ class BraveBrowserView : public BrowserView, FRIEND_TEST_ALL_PREFIXES(SpeedReaderBrowserTest, ToolbarLangs); FRIEND_TEST_ALL_PREFIXES(VerticalTabStripBrowserTest, ExpandedState); FRIEND_TEST_ALL_PREFIXES(VerticalTabStripBrowserTest, ExpandedWidth); + FRIEND_TEST_ALL_PREFIXES(SideBySideEnabledBrowserTest, + BraveMultiContentsViewTest); static void SetDownloadConfirmReturnForTesting(bool allow); @@ -180,6 +182,8 @@ class BraveBrowserView : public BrowserView, bool update_devtools_web_contents) override; void OnWidgetActivationChanged(views::Widget* widget, bool active) override; void GetAccessiblePanes(std::vector* panes) override; + void ShowSplitView() override; + void HideSplitView() override; void StopTabCycling(); void UpdateSearchTabsButtonState(); @@ -203,6 +207,10 @@ class BraveBrowserView : public BrowserView, void UpdateSideBarHorizontalAlignment(); + views::View* contents_separator_for_testing() const { + return contents_separator_; + } + std::unique_ptr vertical_tab_strip_widget_; bool closing_confirm_dialog_activated_ = false; diff --git a/browser/ui/views/split_view/split_view_browsertest.cc b/browser/ui/views/split_view/split_view_browsertest.cc index 5aa846db15d..cd80b7b9bbf 100644 --- a/browser/ui/views/split_view/split_view_browsertest.cc +++ b/browser/ui/views/split_view/split_view_browsertest.cc @@ -8,6 +8,7 @@ #include #include "base/test/run_until.h" +#include "brave/browser/brave_browser_features.h" #include "brave/browser/ui/browser_commands.h" #include "brave/browser/ui/tabs/brave_tab_layout_constants.h" #include "brave/browser/ui/tabs/features.h" @@ -58,7 +59,8 @@ class SideBySideEnabledBrowserTest : public InProcessBrowserTest { public: SideBySideEnabledBrowserTest() { scoped_features_.InitWithFeatures( - /*enabled_features*/ {features::kSideBySide}, {}); + /*enabled_features*/ {features::kSideBySide}, + /*disabled_features*/ {features::kBraveWebViewRoundedCorners}); } ~SideBySideEnabledBrowserTest() override = default; @@ -73,13 +75,22 @@ IN_PROC_BROWSER_TEST_F(SideBySideEnabledBrowserTest, auto* split_view_data = browser()->GetFeatures().split_view_browser_data(); EXPECT_FALSE(!!split_view_data); + auto* browser_view = + BraveBrowserView::From(BrowserView::GetBrowserViewForBrowser(browser())); auto* multi_contents_view = static_cast( - BrowserView::GetBrowserViewForBrowser(browser()) - ->multi_contents_view_for_testing()); + browser_view->multi_contents_view_for_testing()); ASSERT_TRUE(multi_contents_view); + // separator should not be empty when split view is closed. + EXPECT_NE(gfx::Size(), + browser_view->contents_separator_for_testing()->GetPreferredSize()); + chrome::NewSplitTab(browser()); + // separator should be empty when split view is opened. + EXPECT_EQ(gfx::Size(), + browser_view->contents_separator_for_testing()->GetPreferredSize()); + // Check corner radius. auto* start_contents_web_view = multi_contents_view->start_contents_view_for_testing(); diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_view.h b/chromium_src/chrome/browser/ui/views/frame/browser_view.h index 7dfa2bee33f..9d6646058b5 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_view.h +++ b/chromium_src/chrome/browser/ui/views/frame/browser_view.h @@ -50,9 +50,13 @@ #undef LoadAccelerators #endif #define LoadAccelerators virtual LoadAccelerators +#define ShowSplitView virtual ShowSplitView +#define HideSplitView virtual HideSplitView #include "src/chrome/browser/ui/views/frame/browser_view.h" // IWYU pragma: export +#undef HideSplitView +#undef ShowSplitView #undef LoadAccelerators #if BUILDFLAG(IS_WIN) // #pragma pop_macro("LoadAccelerators")