From 3bfbfbe3b5780faecebfa10a664cc11af02f2978 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Fri, 14 Oct 2022 21:51:54 +0900 Subject: [PATCH 01/12] Support hiding window title when using vertical tab strip Add a tab context menu to hide window title when vertical tab strip is enabled. When hiding window title, we can claim title area so that users can use taller contents area. * On Mac and Window, toolbar will take the title area. * On Linux, title area will be as short as possible * can't overlay caption buttons on toolbar for now. Also, we should make it easy to move window when hiding window title as there's almost no non-client area. Thus, enable dragging windows by dragging toolbar area. --- app/brave_generated_resources.grd | 8 +++ browser/ui/BUILD.gn | 2 + browser/ui/tabs/brave_tab_menu_model.cc | 1 + browser/ui/tabs/brave_tab_menu_model.h | 1 + browser/ui/tabs/brave_tab_prefs.cc | 3 + browser/ui/tabs/brave_tab_prefs.h | 1 + .../ui/views/frame/brave_browser_frame_mac.h | 26 +++++++ .../ui/views/frame/brave_browser_frame_mac.mm | 29 ++++++++ .../brave_browser_non_client_frame_view_mac.h | 5 ++ ...brave_browser_non_client_frame_view_mac.mm | 28 +++++++- browser/ui/views/frame/brave_browser_view.cc | 2 +- .../frame/brave_glass_browser_frame_view.cc | 12 ++++ .../frame/brave_glass_browser_frame_view.h | 1 + .../tabs/brave_tab_context_menu_contents.cc | 27 +++++++ .../tabs/brave_tab_context_menu_contents.h | 1 + browser/ui/views/tabs/features.cc | 71 ++++++++++++++++++- browser/ui/views/tabs/features.h | 17 +++++ .../ui/views/toolbar/brave_toolbar_view.cc | 40 +++++++++++ browser/ui/views/toolbar/brave_toolbar_view.h | 6 ++ .../frame/browser_frame_view_layout_linux.cc | 33 ++++++++- .../frame/browser_non_client_frame_view_mac.h | 18 +++++ .../frame/native_browser_frame_factory_mac.mm | 12 ++++ .../app_shim/native_widget_ns_window_bridge.h | 18 +++++ .../native_widget_ns_window_bridge.mm | 28 ++++++++ .../common/native_widget_ns_window.mojom | 11 +++ .../cocoa/native_widget_mac_ns_window_host.h | 17 +++++ .../cocoa/native_widget_mac_ns_window_host.mm | 14 ++++ .../ui/views/widget/native_widget_mac.h | 19 +++++ .../ui/views/widget/native_widget_mac.mm | 14 ++++ chromium_src/ui/views/widget/widget.cc | 21 ++++++ chromium_src/ui/views/widget/widget.h | 23 ++++++ 31 files changed, 502 insertions(+), 7 deletions(-) create mode 100644 browser/ui/views/frame/brave_browser_frame_mac.h create mode 100644 browser/ui/views/frame/brave_browser_frame_mac.mm create mode 100644 chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h create mode 100644 chromium_src/chrome/browser/ui/views/frame/native_browser_frame_factory_mac.mm create mode 100644 chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h create mode 100644 chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm create mode 100644 chromium_src/components/remote_cocoa/common/native_widget_ns_window.mojom create mode 100644 chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.h create mode 100644 chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.mm create mode 100644 chromium_src/ui/views/widget/native_widget_mac.h create mode 100644 chromium_src/ui/views/widget/native_widget_mac.mm create mode 100644 chromium_src/ui/views/widget/widget.cc create mode 100644 chromium_src/ui/views/widget/widget.h diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index 153aae5d1f02..745cc245f939 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -797,15 +797,23 @@ Or change later at $2brave://settings/ext Brave is made available to you under the <a target="_blank" href="$1">Mozilla Public License 2.0</a> (MPL) and includes <a target="_blank" href="$2">open source software</a> under a variety of other licenses. You can read <a target="_blank" href="$3">instructions on how to download and build for yourself</a> the specific <a target="_blank" href="$4">source code used to create this copy</a>. + + Bookmark all tabs... + + Show window title + Bookmark All Tabs... + + Show Window Title + diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 8ac3f3384efb..fa1e7d54651a 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -290,6 +290,8 @@ source_set("ui") { if (is_mac) { sources += [ + "views/frame/brave_browser_frame_mac.h", + "views/frame/brave_browser_frame_mac.mm", "views/frame/brave_browser_non_client_frame_view_mac.h", "views/frame/brave_browser_non_client_frame_view_mac.mm", ] diff --git a/browser/ui/tabs/brave_tab_menu_model.cc b/browser/ui/tabs/brave_tab_menu_model.cc index 6c62caac6208..aee056dc046f 100644 --- a/browser/ui/tabs/brave_tab_menu_model.cc +++ b/browser/ui/tabs/brave_tab_menu_model.cc @@ -55,4 +55,5 @@ void BraveTabMenuModel::Build() { AddSeparator(ui::NORMAL_SEPARATOR); AddItemWithStringId(CommandRestoreTab, GetRestoreTabCommandStringId()); AddItemWithStringId(CommandBookmarkAllTabs, IDS_TAB_CXMENU_BOOKMARK_ALL_TABS); + AddCheckItemWithStringId(CommandShowTitleBar, IDS_TAB_CXMENU_SHOW_TITLE_BAR); } diff --git a/browser/ui/tabs/brave_tab_menu_model.h b/browser/ui/tabs/brave_tab_menu_model.h index 650660438d17..86d50c176ec4 100644 --- a/browser/ui/tabs/brave_tab_menu_model.h +++ b/browser/ui/tabs/brave_tab_menu_model.h @@ -24,6 +24,7 @@ class BraveTabMenuModel : public TabMenuModel { CommandStart = TabStripModel::CommandLast, CommandRestoreTab, CommandBookmarkAllTabs, + CommandShowTitleBar, CommandLast, }; diff --git a/browser/ui/tabs/brave_tab_prefs.cc b/browser/ui/tabs/brave_tab_prefs.cc index b11cb68a8e06..7923db9932ed 100644 --- a/browser/ui/tabs/brave_tab_prefs.cc +++ b/browser/ui/tabs/brave_tab_prefs.cc @@ -12,10 +12,13 @@ namespace brave_tabs { const char kTabHoverMode[] = "brave.tabs.hover_mode"; const char kVerticalTabsCollapsed[] = "brave.tabs.vertical_tabs_collapsed"; +const char kVerticalTabsShowTitleOnWindow[] = + "brave.tabs.vertical_tabs_show_title_on_window"; void RegisterBraveProfilePrefs(PrefRegistrySimple* registry) { registry->RegisterIntegerPref(kTabHoverMode, TabHoverMode::CARD); registry->RegisterBooleanPref(kVerticalTabsCollapsed, false); + registry->RegisterBooleanPref(kVerticalTabsShowTitleOnWindow, true); } bool AreTooltipsEnabled(PrefService* prefs) { diff --git a/browser/ui/tabs/brave_tab_prefs.h b/browser/ui/tabs/brave_tab_prefs.h index 8f1ebbeaabd1..6a4efa4bdaf8 100644 --- a/browser/ui/tabs/brave_tab_prefs.h +++ b/browser/ui/tabs/brave_tab_prefs.h @@ -15,6 +15,7 @@ enum TabHoverMode { TOOLTIP = 0, CARD = 1, CARD_WITH_PREVIEW = 2 }; extern const char kTabHoverMode[]; extern const char kVerticalTabsCollapsed[]; +extern const char kVerticalTabsShowTitleOnWindow[]; void RegisterBraveProfilePrefs(PrefRegistrySimple* registry); diff --git a/browser/ui/views/frame/brave_browser_frame_mac.h b/browser/ui/views/frame/brave_browser_frame_mac.h new file mode 100644 index 000000000000..38e3584fdf56 --- /dev/null +++ b/browser/ui/views/frame/brave_browser_frame_mac.h @@ -0,0 +1,26 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_ +#define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_ + +#include "chrome/browser/ui/views/frame/browser_frame_mac.h" + +class Browser; + +class BraveBrowserFrameMac : public BrowserFrameMac { + public: + BraveBrowserFrameMac(BrowserFrame* browser_frame, BrowserView* browser_view); + ~BraveBrowserFrameMac() override; + + // BrowserFrameMac: + void GetWindowFrameTitlebarHeight(bool* override_titlebar_height, + float* titlebar_height) override; + + private: + raw_ptr browser_; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_FRAME_MAC_H_ diff --git a/browser/ui/views/frame/brave_browser_frame_mac.mm b/browser/ui/views/frame/brave_browser_frame_mac.mm new file mode 100644 index 000000000000..02441143d930 --- /dev/null +++ b/browser/ui/views/frame/brave_browser_frame_mac.mm @@ -0,0 +1,29 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/frame/brave_browser_frame_mac.h" + +#include "brave/browser/ui/views/tabs/features.h" +#include "chrome/browser/ui/views/frame/browser_view.h" + +BraveBrowserFrameMac::BraveBrowserFrameMac(BrowserFrame* browser_frame, + BrowserView* browser_view) + : BrowserFrameMac(browser_frame, browser_view), + browser_(browser_view->browser()) {} + +BraveBrowserFrameMac::~BraveBrowserFrameMac() = default; + +void BraveBrowserFrameMac::GetWindowFrameTitlebarHeight( + bool* override_titlebar_height, + float* titlebar_height) { + // Don't override titlebar height if we supports vertical tab strip so that + // it can overlay our client view. The visibility of title bar will be + // controlled by BrowserNonClientFrameViewMac::UpdateWindowTitleVisibility. + if (browser_->is_type_normal() && tabs::features::ShouldShowVerticalTabs()) + return; + + return BrowserFrameMac::GetWindowFrameTitlebarHeight(override_titlebar_height, + titlebar_height); +} diff --git a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h index 49bb4a263698..7c140d69051b 100644 --- a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h +++ b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h @@ -24,11 +24,16 @@ class BraveBrowserNonClientFrameViewMac : public BrowserNonClientFrameViewMac { const BraveBrowserNonClientFrameViewMac&) = delete; private: + bool ShouldShowWindowTitleForVerticalTabs() const; + void UpdateWindowTitleVisibility(); + // BrowserNonClientFrameViewMac overrides: void OnPaint(gfx::Canvas* canvas) override; int GetTopInset(bool restored) const override; std::unique_ptr frame_graphic_; + + BooleanPrefMember show_title_bar_on_vertical_tabs_; }; #endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_BROWSER_NON_CLIENT_FRAME_VIEW_MAC_H_ diff --git a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm index 09b3de506ff0..a7526611be49 100644 --- a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm +++ b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm @@ -7,9 +7,11 @@ #include "brave/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h" +#include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/views/frame/brave_window_frame_graphic.h" #include "brave/browser/ui/views/tabs/features.h" #include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/view_ids.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/rect.h" @@ -20,6 +22,15 @@ : BrowserNonClientFrameViewMac(frame, browser_view) { frame_graphic_ = std::make_unique( browser_view->browser()->profile()); + + if (tabs::features::ShouldShowVerticalTabs()) { + show_title_bar_on_vertical_tabs_.Init( + brave_tabs::kVerticalTabsShowTitleOnWindow, + browser_view->browser()->profile()->GetOriginalProfile()->GetPrefs(), + base::BindRepeating( + &BraveBrowserNonClientFrameViewMac::UpdateWindowTitleVisibility, + base::Unretained(this))); + } } BraveBrowserNonClientFrameViewMac::~BraveBrowserNonClientFrameViewMac() = default; @@ -41,10 +52,23 @@ } int BraveBrowserNonClientFrameViewMac::GetTopInset(bool restored) const { - if (tabs::features::ShouldShowVerticalTabs()) { + if (ShouldShowWindowTitleForVerticalTabs()) { // Set minimum top inset to show caption buttons on frame. - return 18; + return 30; } return BrowserNonClientFrameViewMac::GetTopInset(restored); } + +bool BraveBrowserNonClientFrameViewMac::ShouldShowWindowTitleForVerticalTabs() + const { + return tabs::features::ShouldShowWindowTitleForVerticalTabs( + browser_view()->browser()); +} + +void BraveBrowserNonClientFrameViewMac::UpdateWindowTitleVisibility() { + if (!browser_view()->browser()->is_type_normal()) + return; + + frame()->SetWindowTitleVisibility(ShouldShowWindowTitleForVerticalTabs()); +} diff --git a/browser/ui/views/frame/brave_browser_view.cc b/browser/ui/views/frame/brave_browser_view.cc index ab37f78cd676..aac5b7dc3f78 100644 --- a/browser/ui/views/frame/brave_browser_view.cc +++ b/browser/ui/views/frame/brave_browser_view.cc @@ -502,7 +502,7 @@ bool BraveBrowserView::ShouldShowWindowTitle() const { if (BrowserView::ShouldShowWindowTitle()) return true; - if (tabs::features::ShouldShowVerticalTabs() && browser_->is_type_normal()) + if (tabs::features::ShouldShowWindowTitleForVerticalTabs(browser())) return true; return false; diff --git a/browser/ui/views/frame/brave_glass_browser_frame_view.cc b/browser/ui/views/frame/brave_glass_browser_frame_view.cc index a7e031997ae9..f2a2888fd037 100644 --- a/browser/ui/views/frame/brave_glass_browser_frame_view.cc +++ b/browser/ui/views/frame/brave_glass_browser_frame_view.cc @@ -6,6 +6,7 @@ #include "brave/browser/ui/views/frame/brave_glass_browser_frame_view.h" #include "brave/browser/ui/views/frame/brave_window_frame_graphic.h" +#include "brave/browser/ui/views/tabs/features.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "ui/gfx/geometry/insets.h" @@ -36,3 +37,14 @@ void BraveGlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) { } frame_graphic_->Paint(canvas, bounds_to_frame_graphic); } + +int BraveGlassBrowserFrameView::GetTopInset(bool restored) const { + if (browser_view()->browser()->is_type_normal() && + tabs::features::ShouldShowVerticalTabs()) { + if (!tabs::features::ShouldShowWindowTitleForVerticalTabs( + browser_view()->browser())) + return 0; + } + + return GlassBrowserFrameView::GetTopInset(restored); +} diff --git a/browser/ui/views/frame/brave_glass_browser_frame_view.h b/browser/ui/views/frame/brave_glass_browser_frame_view.h index 813fdb2cf204..11ca54ff33e3 100644 --- a/browser/ui/views/frame/brave_glass_browser_frame_view.h +++ b/browser/ui/views/frame/brave_glass_browser_frame_view.h @@ -24,6 +24,7 @@ class BraveGlassBrowserFrameView : public GlassBrowserFrameView { private: // GlassBrowserFrameView overrides: void OnPaint(gfx::Canvas* canvas) override; + int GetTopInset(bool restored) const override; std::unique_ptr frame_graphic_; }; diff --git a/browser/ui/views/tabs/brave_tab_context_menu_contents.cc b/browser/ui/views/tabs/brave_tab_context_menu_contents.cc index e22b49ab892a..65528c2e2b0c 100644 --- a/browser/ui/views/tabs/brave_tab_context_menu_contents.cc +++ b/browser/ui/views/tabs/brave_tab_context_menu_contents.cc @@ -6,7 +6,9 @@ #include "brave/browser/ui/views/tabs/brave_tab_context_menu_contents.h" #include "brave/browser/ui/tabs/brave_tab_menu_model.h" +#include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/views/tabs/brave_browser_tab_strip_controller.h" +#include "brave/browser/ui/views/tabs/features.h" #include "chrome/browser/defaults.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sessions/tab_restore_service_factory.h" @@ -48,6 +50,14 @@ void BraveTabContextMenuContents::RunMenuAt( } bool BraveTabContextMenuContents::IsCommandIdChecked(int command_id) const { + if (command_id == BraveTabMenuModel::CommandShowTitleBar) { + return controller_->browser() + ->profile() + ->GetOriginalProfile() + ->GetPrefs() + ->GetBoolean(brave_tabs::kVerticalTabsShowTitleOnWindow); + } + return false; } @@ -60,6 +70,13 @@ bool BraveTabContextMenuContents::IsCommandIdEnabled(int command_id) const { tab_); } +bool BraveTabContextMenuContents::IsCommandIdVisible(int command_id) const { + if (command_id == BraveTabMenuModel::CommandShowTitleBar) + return tabs::features::ShouldShowVerticalTabs(); + + return ui::SimpleMenuModel::Delegate::IsCommandIdVisible(command_id); +} + bool BraveTabContextMenuContents::GetAcceleratorForCommandId( int command_id, ui::Accelerator* accelerator) const { @@ -99,6 +116,8 @@ bool BraveTabContextMenuContents::IsBraveCommandIdEnabled( chrome::CanBookmarkAllTabs(browser_); } break; + case BraveTabMenuModel::CommandShowTitleBar: + return true; default: NOTREACHED(); break; @@ -115,6 +134,14 @@ void BraveTabContextMenuContents::ExecuteBraveCommand(int command_id) { case BraveTabMenuModel::CommandBookmarkAllTabs: chrome::BookmarkAllTabs(browser_); return; + case BraveTabMenuModel::CommandShowTitleBar: { + auto* browser = controller_->browser(); + browser->profile()->GetOriginalProfile()->GetPrefs()->SetBoolean( + brave_tabs::kVerticalTabsShowTitleOnWindow, + !IsCommandIdChecked(command_id)); + BrowserView::GetBrowserViewForBrowser(browser)->InvalidateLayout(); + return; + } default: NOTREACHED(); return; diff --git a/browser/ui/views/tabs/brave_tab_context_menu_contents.h b/browser/ui/views/tabs/brave_tab_context_menu_contents.h index da570cb8ad8b..30c09d705f52 100644 --- a/browser/ui/views/tabs/brave_tab_context_menu_contents.h +++ b/browser/ui/views/tabs/brave_tab_context_menu_contents.h @@ -42,6 +42,7 @@ class BraveTabContextMenuContents : public ui::SimpleMenuModel::Delegate { // ui::SimpleMenuModel::Delegate overrides: bool IsCommandIdChecked(int command_id) const override; bool IsCommandIdEnabled(int command_id) const override; + bool IsCommandIdVisible(int command_id) const override; bool GetAcceleratorForCommandId(int command_id, ui::Accelerator* accelerator) const override; void ExecuteCommand(int command_id, int event_flags) override; diff --git a/browser/ui/views/tabs/features.cc b/browser/ui/views/tabs/features.cc index 75758a4fc4ca..7e819d77439e 100644 --- a/browser/ui/views/tabs/features.cc +++ b/browser/ui/views/tabs/features.cc @@ -5,7 +5,22 @@ #include "brave/browser/ui/views/tabs/features.h" -#include "base/feature_list.h" +#include "brave/browser/ui/tabs/brave_tab_prefs.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "components/prefs/pref_service.h" + +#if !BUILDFLAG(IS_MAC) +#include "chrome/browser/ui/frame/window_frame_util.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h" +#include "ui/base/theme_provider.h" +#include "ui/views/resources/grit/views_resources.h" +#endif + +#if BUILDFLAG(IS_LINUX) +#include "ui/views/window/caption_button_layout_constants.h" +#endif namespace tabs { namespace features { @@ -19,5 +34,59 @@ bool ShouldShowVerticalTabs() { return base::FeatureList::IsEnabled(features::kBraveVerticalTabs); } +bool ShouldShowWindowTitleForVerticalTabs(const Browser* browser) { + DCHECK(browser); + + if (!ShouldShowVerticalTabs()) + return false; + + if (!browser->is_type_normal()) + return false; + + return browser->profile()->GetOriginalProfile()->GetPrefs()->GetBoolean( + brave_tabs::kVerticalTabsShowTitleOnWindow); +} + +std::pair GetLeadingTrailingCaptionButtonWidth( + const BrowserFrame* frame) { +#if BUILDFLAG(IS_MAC) + // On Mac, window caption buttons are drawn by the system. + return {80, 0}; +#elif BUILDFLAG(IS_LINUX) + // On Linux, we can't overlay caption buttons on toolbar. + return {0, 0}; +#elif BUILDFLAG(IS_WIN) + if (frame->ShouldUseNativeFrame()) { + // In this case, we usd GlassBrowserFrameView. Native frame will be set to + // the HWND and and GlassBrowserFrameView will draw frame and window caption + // button. + auto size = WindowFrameUtil::GetWindows10GlassCaptionButtonAreaSize(); + if (WindowFrameUtil::IsWin10TabSearchCaptionButtonEnabled( + BrowserView::GetBrowserViewForNativeWindow(frame->GetNativeWindow()) + ->browser())) { + size.set_width( + size.width() + WindowFrameUtil::kWindows10GlassCaptionButtonWidth + + WindowFrameUtil::kWindows10GlassCaptionButtonVisualSpacing); + } + return {0, size.width()}; + } + + // In this case, we use OpaqueBrowserFrameView. OpaqueBrowserFrameView has + // two types of frame button per platform but on Window, it uses image + // buttons. See OpaqueBrowserFrameView::GetFrameButtonStyle(). + int width = 0; + // Uses image icons + const ui::ThemeProvider* tp = frame->GetThemeProvider(); + DCHECK(tp); + for (auto image_id : {IDR_MINIMIZE, IDR_MAXIMIZE, IDR_CLOSE}) { + if (const gfx::ImageSkia* image = tp->GetImageSkiaNamed(image_id)) + width += image->width(); + } + return {0, width}; +#else +#error "not handled platform" +#endif +} + } // namespace features } // namespace tabs diff --git a/browser/ui/views/tabs/features.h b/browser/ui/views/tabs/features.h index 08064b25fa3b..41109debbe67 100644 --- a/browser/ui/views/tabs/features.h +++ b/browser/ui/views/tabs/features.h @@ -6,12 +6,21 @@ #ifndef BRAVE_BROWSER_UI_VIEWS_TABS_FEATURES_H_ #define BRAVE_BROWSER_UI_VIEWS_TABS_FEATURES_H_ +#include + #include "base/compiler_specific.h" namespace base { struct LOGICALLY_CONST Feature; } // namespace base +namespace views { +class View; +} // namespace views + +class Browser; +class BrowserFrame; + namespace tabs { namespace features { @@ -20,6 +29,14 @@ extern const base::Feature kBraveVerticalTabs; // Returns true when users chose to use vertical tabs bool ShouldShowVerticalTabs(); +// Returns true when we should show window title on window frame when vertical +// tab strip is enabled. +bool ShouldShowWindowTitleForVerticalTabs(const Browser* browser); + +// Returns window caption buttons' width based on the current platform +std::pair GetLeadingTrailingCaptionButtonWidth( + const BrowserFrame* frame); + } // namespace features } // namespace tabs diff --git a/browser/ui/views/toolbar/brave_toolbar_view.cc b/browser/ui/views/toolbar/brave_toolbar_view.cc index e5240069789d..f42e85cee377 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view.cc +++ b/browser/ui/views/toolbar/brave_toolbar_view.cc @@ -12,6 +12,8 @@ #include "base/bind.h" #include "brave/app/brave_command_ids.h" #include "brave/browser/brave_wallet/brave_wallet_context_utils.h" +#include "brave/browser/ui/tabs/brave_tab_prefs.h" +#include "brave/browser/ui/views/tabs/features.h" #include "brave/browser/ui/views/toolbar/bookmark_button.h" #include "brave/browser/ui/views/toolbar/wallet_button.h" #include "brave/components/brave_vpn/buildflags/buildflags.h" @@ -28,8 +30,10 @@ #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h" #include "components/bookmarks/common/bookmark_pref_names.h" #include "components/prefs/pref_service.h" +#include "ui/base/hit_test.h" #include "ui/base/window_open_disposition.h" #include "ui/events/event.h" +#include "ui/views/window/hit_test_utils.h" #if BUILDFLAG(ENABLE_BRAVE_VPN) #include "brave/browser/brave_vpn/vpn_utils.h" @@ -115,6 +119,9 @@ BraveToolbarView::~BraveToolbarView() = default; void BraveToolbarView::Init() { ToolbarView::Init(); + // This will allow us to move this window by dragging toolbar + views::SetHitTestComponent(this, HTCAPTION); + // For non-normal mode, we don't have to more. if (display_mode_ != DisplayMode::NORMAL) { brave_initialized_ = true; @@ -143,6 +150,16 @@ void BraveToolbarView::Init() { base::BindRepeating(&BraveToolbarView::OnLocationBarIsWideChanged, base::Unretained(this))); + if (tabs::features::ShouldShowVerticalTabs()) { + show_title_bar_on_vertical_tabs_.Init( + brave_tabs::kVerticalTabsShowTitleOnWindow, + profile->GetOriginalProfile()->GetPrefs(), + base::BindRepeating( + &BraveToolbarView::OnShowTitleBarOnVerticalTabsChanged, + base::Unretained(this))); + OnShowTitleBarOnVerticalTabsChanged(); + } + const auto callback = [](Browser* browser, int command, const ui::Event& event) { chrome::ExecuteCommandWithDisposition( @@ -265,6 +282,18 @@ void BraveToolbarView::UpdateBookmarkVisibility() { show_bookmarks_button_.GetValue()); } +void BraveToolbarView::OnShowTitleBarOnVerticalTabsChanged() { + if (tabs::features::ShouldShowWindowTitleForVerticalTabs(browser())) { + SetBorder(nullptr); + } else { + auto [leading, trailing] = + tabs::features::GetLeadingTrailingCaptionButtonWidth( + browser_view_->frame()); + SetBorder(views::CreateEmptyBorder( + gfx::Insets().set_left(leading).set_right(trailing))); + } +} + void BraveToolbarView::ShowBookmarkBubble( const GURL& url, bool already_bookmarked, @@ -284,6 +313,17 @@ void BraveToolbarView::ShowBookmarkBubble( browser_->profile(), url, already_bookmarked); } +void BraveToolbarView::ViewHierarchyChanged( + const views::ViewHierarchyChangedDetails& details) { + ToolbarView::ViewHierarchyChanged(details); + + if (details.is_add && details.parent == this) { + // Mark children of this view as client area so that they are not perceived + // as client area. + views::SetHitTestComponent(details.child, HTCLIENT); + } +} + void BraveToolbarView::Layout() { ToolbarView::Layout(); diff --git a/browser/ui/views/toolbar/brave_toolbar_view.h b/browser/ui/views/toolbar/brave_toolbar_view.h index f5fb097fddd3..abe1239405ca 100644 --- a/browser/ui/views/toolbar/brave_toolbar_view.h +++ b/browser/ui/views/toolbar/brave_toolbar_view.h @@ -44,12 +44,15 @@ class BraveToolbarView : public ToolbarView, void ShowBookmarkBubble(const GURL& url, bool already_bookmarked, bookmarks::BookmarkBubbleObserver* observer) override; + void ViewHierarchyChanged( + const views::ViewHierarchyChangedDetails& details) override; private: void LoadImages() override; void ResetLocationBarBounds(); void ResetButtonBounds(); void UpdateBookmarkVisibility(); + void OnShowTitleBarOnVerticalTabsChanged(); // ProfileAttributesStorage::Observer: void OnProfileAdded(const base::FilePath& profile_path) override; @@ -70,6 +73,9 @@ class BraveToolbarView : public ToolbarView, BooleanPrefMember show_bookmarks_button_; BooleanPrefMember location_bar_is_wide_; + + BooleanPrefMember show_title_bar_on_vertical_tabs_; + // Whether this toolbar has been initialized. bool brave_initialized_ = false; // Tracks profile count to determine whether profile switcher should be shown. diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc index db3b10a0a638..3b67f22ea3d4 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc +++ b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc @@ -3,14 +3,41 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "base/check_is_test.h" #include "brave/browser/ui/views/tabs/features.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "ui/views/window/caption_button_layout_constants.h" #include "src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc" int BrowserFrameViewLayoutLinux::NonClientTopHeight(bool restored) const { - if (tabs::features::ShouldShowVerticalTabs()) { - // Set minimum height to show caption buttons. - return 50; + if (!view_) { + CHECK_IS_TEST(); + return OpaqueBrowserFrameViewLayout::NonClientTopHeight(restored); + } + + if (view_->browser_view()->browser()->is_type_normal() && + tabs::features::ShouldShowVerticalTabs()) { + if (!view_->ShouldShowCaptionButtons()) { + // In this case, the window manager might be forcibly providing system + // window title or it's in fullscreen mode. We shouldn't show title bar + // in this case. + return OpaqueBrowserFrameViewLayout::NonClientTopHeight(restored); + } + + if (tabs::features::ShouldShowWindowTitleForVerticalTabs( + view_->browser_view()->browser())) { + return 30; + } + + // TODO(sko) For now, I couldn't find a way to overlay caption buttons + // on toolbar. Once it gets possible, we shouldn't reserve non client top + // height. + if (view_->ShouldShowCaptionButtons()) { + // Uses the same caption height defined in + // c/b/u/v/frame/opaque_browser_frame_view_layout.cc + return 18; + } } return OpaqueBrowserFrameViewLayout::NonClientTopHeight(restored); diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h b/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h new file mode 100644 index 000000000000..4205e315c551 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h @@ -0,0 +1,18 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_NON_CLIENT_FRAME_VIEW_MAC_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_NON_CLIENT_FRAME_VIEW_MAC_H_ + +#define PaintThemedFrame \ + PaintThemedFrame_Unused() {} \ + friend class BraveBrowserNonClientFrameViewMac; \ + void PaintThemedFrame + +#include "src/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h" + +#undef PaintThemedFrame + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_NON_CLIENT_FRAME_VIEW_MAC_H_ diff --git a/chromium_src/chrome/browser/ui/views/frame/native_browser_frame_factory_mac.mm b/chromium_src/chrome/browser/ui/views/frame/native_browser_frame_factory_mac.mm new file mode 100644 index 000000000000..3009d2263c6a --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/frame/native_browser_frame_factory_mac.mm @@ -0,0 +1,12 @@ +/* Copyright 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/frame/brave_browser_frame_mac.h" + +#define BrowserFrameMac BraveBrowserFrameMac + +#include "src/chrome/browser/ui/views/frame/native_browser_frame_factory_mac.mm" + +#undef BrowserFrameMac diff --git a/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h b/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h new file mode 100644 index 000000000000..def28564a064 --- /dev/null +++ b/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h @@ -0,0 +1,18 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_REMOTE_COCOA_APP_SHIM_NATIVE_WIDGET_NS_WINDOW_BRIDGE_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_REMOTE_COCOA_APP_SHIM_NATIVE_WIDGET_NS_WINDOW_BRIDGE_H_ + +// Override a method from our NativeWidgetNSWindow mojo extension +#define OnSizeChanged \ + SetWindowTitleVisibility(bool visible) override; \ + void OnSizeChanged + +#include "src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h" + +#undef OnSizeChanged + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_REMOTE_COCOA_APP_SHIM_NATIVE_WIDGET_NS_WINDOW_BRIDGE_H_ diff --git a/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm b/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm new file mode 100644 index 000000000000..3871e71dee08 --- /dev/null +++ b/chromium_src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm @@ -0,0 +1,28 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "src/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm" + +namespace remote_cocoa { + +void NativeWidgetNSWindowBridge::SetWindowTitleVisibility(bool visible) { + NSUInteger style_mask = [window_ styleMask]; + if (visible) + style_mask |= NSWindowStyleMaskTitled; + else + style_mask &= ~NSWindowStyleMaskTitled; + + [window_ setStyleMask:style_mask]; + + // Sometimes title is not visible until window is resized. In order to avoid + // this, reset title to force it to be visible. + if (visible) { + NSString* title = window_.get().title; + window_.get().title = @""; + window_.get().title = title; + } +} + +} // namespace remote_cocoa diff --git a/chromium_src/components/remote_cocoa/common/native_widget_ns_window.mojom b/chromium_src/components/remote_cocoa/common/native_widget_ns_window.mojom new file mode 100644 index 000000000000..8dafb1654935 --- /dev/null +++ b/chromium_src/components/remote_cocoa/common/native_widget_ns_window.mojom @@ -0,0 +1,11 @@ +// Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. + +module remote_cocoa.mojom; + +[BraveExtend] +interface NativeWidgetNSWindow { + SetWindowTitleVisibility(bool visible); +}; diff --git a/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.h b/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.h new file mode 100644 index 000000000000..a51013bbb0da --- /dev/null +++ b/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_UI_VIEWS_COCOA_NATIVE_WIDGET_MAC_NS_WINDOW_HOST_H_ +#define BRAVE_CHROMIUM_SRC_UI_VIEWS_COCOA_NATIVE_WIDGET_MAC_NS_WINDOW_HOST_H_ + +#define OnWidgetInitDone \ + SetWindowTitleVisibility(bool visible); \ + void OnWidgetInitDone + +#include "src/ui/views/cocoa/native_widget_mac_ns_window_host.h" + +#undef OnWidgetInitDone + +#endif // BRAVE_CHROMIUM_SRC_UI_VIEWS_COCOA_NATIVE_WIDGET_MAC_NS_WINDOW_HOST_H_ diff --git a/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.mm b/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.mm new file mode 100644 index 000000000000..8c549ad77fce --- /dev/null +++ b/chromium_src/ui/views/cocoa/native_widget_mac_ns_window_host.mm @@ -0,0 +1,14 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "src/ui/views/cocoa/native_widget_mac_ns_window_host.mm" + +namespace views { + +void NativeWidgetMacNSWindowHost::SetWindowTitleVisibility(bool visible) { + GetNSWindowMojo()->SetWindowTitleVisibility(visible); +} + +} // namespace views diff --git a/chromium_src/ui/views/widget/native_widget_mac.h b/chromium_src/ui/views/widget/native_widget_mac.h new file mode 100644 index 000000000000..7b592a5d3d09 --- /dev/null +++ b/chromium_src/ui/views/widget/native_widget_mac.h @@ -0,0 +1,19 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_NATIVE_WIDGET_MAC_H_ +#define BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_NATIVE_WIDGET_MAC_H_ + +#include "ui/views/widget/native_widget_private.h" + +#define SetWindowIcons \ + SetWindowTitleVisibility(bool visible); \ + void SetWindowIcons + +#include "src/ui/views/widget/native_widget_mac.h" + +#undef SetWindowIcons + +#endif // BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_NATIVE_WIDGET_MAC_H_ diff --git a/chromium_src/ui/views/widget/native_widget_mac.mm b/chromium_src/ui/views/widget/native_widget_mac.mm new file mode 100644 index 000000000000..0477cd65b6c3 --- /dev/null +++ b/chromium_src/ui/views/widget/native_widget_mac.mm @@ -0,0 +1,14 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "src/ui/views/widget/native_widget_mac.mm" + +namespace views { + +void NativeWidgetMac::SetWindowTitleVisibility(bool visible) { + GetNSWindowMojo()->SetWindowTitleVisibility(visible); +} + +} // namespace views diff --git a/chromium_src/ui/views/widget/widget.cc b/chromium_src/ui/views/widget/widget.cc new file mode 100644 index 000000000000..aa7f9d765709 --- /dev/null +++ b/chromium_src/ui/views/widget/widget.cc @@ -0,0 +1,21 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "src/ui/views/widget/widget.cc" + +#if BUILDFLAG(IS_MAC) + +#include "ui/views/widget/native_widget_mac.h" + +namespace views { + +void Widget::SetWindowTitleVisibility(bool visible) { + static_cast(native_widget_) + ->SetWindowTitleVisibility(visible); +} + +} // namespace views + +#endif // defined(OS_MAC) diff --git a/chromium_src/ui/views/widget/widget.h b/chromium_src/ui/views/widget/widget.h new file mode 100644 index 000000000000..65e7e3e1489f --- /dev/null +++ b/chromium_src/ui/views/widget/widget.h @@ -0,0 +1,23 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_WIDGET_H_ +#define BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_WIDGET_H_ + +#include "build/build_config.h" + +#if BUILDFLAG(IS_MAC) +#define UnlockPaintAsActive \ + SetWindowTitleVisibility(bool visible); \ + void UnlockPaintAsActive +#else +#define UnlockPaintAsActive UnlockPaintAsActive +#endif + +#include "src/ui/views/widget/widget.h" + +#undef UnlockPaintAsActive + +#endif // BRAVE_CHROMIUM_SRC_UI_VIEWS_WIDGET_WIDGET_H_ From 88c9f2b6b321aab7a2cd96fb16e1aba6ce608104 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Fri, 14 Oct 2022 21:51:54 +0900 Subject: [PATCH 02/12] Support hiding window title when using vertical tab strip Add a tab context menu to hide window title when vertical tab strip is enabled. When hiding window title, we can claim title area so that users can use taller contents area. * On Mac and Window, toolbar will take the title area. * On Linux, title area will be as short as possible * can't overlay caption buttons on toolbar for now. Also, we should make it easy to move window when hiding window title as there's almost no non-client area. Thus, enable dragging windows by dragging toolbar area. --- browser/ui/BUILD.gn | 2 ++ .../brave_browser_non_client_frame_view_mac.h | 1 + ...brave_browser_non_client_frame_view_mac.mm | 12 ++++++++++ .../frame/brave_glass_browser_frame_view.cc | 10 ++++++++ .../frame/brave_glass_browser_frame_view.h | 1 + .../frame/brave_non_client_hit_test_helper.cc | 23 ++++++++++++++++++ .../frame/brave_non_client_hit_test_helper.h | 24 +++++++++++++++++++ .../frame/brave_opaque_browser_frame_view.cc | 11 +++++++++ .../frame/brave_opaque_browser_frame_view.h | 1 + 9 files changed, 85 insertions(+) create mode 100644 browser/ui/views/frame/brave_non_client_hit_test_helper.cc create mode 100644 browser/ui/views/frame/brave_non_client_hit_test_helper.h diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index fa1e7d54651a..7f5935273b60 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -217,6 +217,8 @@ source_set("ui") { "views/frame/brave_browser_view.h", "views/frame/brave_browser_view_layout.cc", "views/frame/brave_browser_view_layout.h", + "views/frame/brave_non_client_hit_test_helper.cc", + "views/frame/brave_non_client_hit_test_helper.h", "views/frame/brave_opaque_browser_frame_view.cc", "views/frame/brave_opaque_browser_frame_view.h", "views/frame/brave_window_frame_graphic.cc", diff --git a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h index 7c140d69051b..f96b35075b89 100644 --- a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h +++ b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h @@ -30,6 +30,7 @@ class BraveBrowserNonClientFrameViewMac : public BrowserNonClientFrameViewMac { // BrowserNonClientFrameViewMac overrides: void OnPaint(gfx::Canvas* canvas) override; int GetTopInset(bool restored) const override; + int NonClientHitTest(const gfx::Point& point) override; std::unique_ptr frame_graphic_; diff --git a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm index a7526611be49..78316125a038 100644 --- a/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm +++ b/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.mm @@ -8,11 +8,13 @@ #include "brave/browser/ui/views/frame/brave_browser_non_client_frame_view_mac.h" #include "brave/browser/ui/tabs/brave_tab_prefs.h" +#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h" #include "brave/browser/ui/views/frame/brave_window_frame_graphic.h" #include "brave/browser/ui/views/tabs/features.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/view_ids.h" #include "chrome/browser/ui/views/frame/browser_view.h" +#include "ui/base/hit_test.h" #include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/rect.h" #include "ui/gfx/scoped_canvas.h" @@ -72,3 +74,13 @@ frame()->SetWindowTitleVisibility(ShouldShowWindowTitleForVerticalTabs()); } + +int BraveBrowserNonClientFrameViewMac::NonClientHitTest( + const gfx::Point& point) { + if (auto res = brave::NonClientHitTest(browser_view(), point); + res != HTNOWHERE) { + return res; + } + + return BrowserNonClientFrameViewMac::NonClientHitTest(point); +} diff --git a/browser/ui/views/frame/brave_glass_browser_frame_view.cc b/browser/ui/views/frame/brave_glass_browser_frame_view.cc index f2a2888fd037..c55687cebe9e 100644 --- a/browser/ui/views/frame/brave_glass_browser_frame_view.cc +++ b/browser/ui/views/frame/brave_glass_browser_frame_view.cc @@ -5,10 +5,12 @@ #include "brave/browser/ui/views/frame/brave_glass_browser_frame_view.h" +#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h" #include "brave/browser/ui/views/frame/brave_window_frame_graphic.h" #include "brave/browser/ui/views/tabs/features.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/views/frame/browser_view.h" +#include "ui/base/hit_test.h" #include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/rect.h" #include "ui/gfx/scoped_canvas.h" @@ -48,3 +50,11 @@ int BraveGlassBrowserFrameView::GetTopInset(bool restored) const { return GlassBrowserFrameView::GetTopInset(restored); } +int BraveGlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) { + if (auto res = brave::NonClientHitTest(browser_view(), point); + res != HTNOWHERE) { + return res; + } + + return GlassBrowserFrameView::NonClientHitTest(point); +} diff --git a/browser/ui/views/frame/brave_glass_browser_frame_view.h b/browser/ui/views/frame/brave_glass_browser_frame_view.h index 11ca54ff33e3..96b1b2f49d5b 100644 --- a/browser/ui/views/frame/brave_glass_browser_frame_view.h +++ b/browser/ui/views/frame/brave_glass_browser_frame_view.h @@ -25,6 +25,7 @@ class BraveGlassBrowserFrameView : public GlassBrowserFrameView { // GlassBrowserFrameView overrides: void OnPaint(gfx::Canvas* canvas) override; int GetTopInset(bool restored) const override; + int NonClientHitTest(const gfx::Point& point) override; std::unique_ptr frame_graphic_; }; diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper.cc b/browser/ui/views/frame/brave_non_client_hit_test_helper.cc new file mode 100644 index 000000000000..57f7856a83fb --- /dev/null +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper.cc @@ -0,0 +1,23 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h" + +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "ui/base/hit_test.h" +#include "ui/views/window/hit_test_utils.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" + +namespace brave { + +int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point) { + if (!browser_view->toolbar() || !browser_view->toolbar()->GetVisible()) + return HTNOWHERE; + + return views::GetHitTestComponent(browser_view->toolbar(), point); +} + +} // brave + diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper.h b/browser/ui/views/frame/brave_non_client_hit_test_helper.h new file mode 100644 index 000000000000..a6664244e459 --- /dev/null +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper.h @@ -0,0 +1,24 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ +#define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ + +namespace gfx { +class Point; +} // gfx + +class BrowserView; + +namespace brave { + +// Helper function to set additional draggable area in client view. +// Returns HTNOWHERE if the point is not what we're interested in. In that +// case, caller should depend on the default behavior. +int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point); + +} // brave + +#endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ diff --git a/browser/ui/views/frame/brave_opaque_browser_frame_view.cc b/browser/ui/views/frame/brave_opaque_browser_frame_view.cc index 2e52863d9d95..7fe9452fae08 100644 --- a/browser/ui/views/frame/brave_opaque_browser_frame_view.cc +++ b/browser/ui/views/frame/brave_opaque_browser_frame_view.cc @@ -7,10 +7,12 @@ #include +#include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h" #include "brave/browser/ui/views/frame/brave_window_frame_graphic.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h" +#include "ui/base/hit_test.h" #include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/rect.h" #include "ui/gfx/scoped_canvas.h" @@ -40,3 +42,12 @@ void BraveOpaqueBrowserFrameView::OnPaint(gfx::Canvas* canvas) { } frame_graphic_->Paint(canvas, bounds_to_frame_graphic); } + +int BraveOpaqueBrowserFrameView::NonClientHitTest(const gfx::Point& point) { + if (auto res = brave::NonClientHitTest(browser_view(), point); + res != HTNOWHERE) { + return res; + } + + return OpaqueBrowserFrameView::NonClientHitTest(point); +} diff --git a/browser/ui/views/frame/brave_opaque_browser_frame_view.h b/browser/ui/views/frame/brave_opaque_browser_frame_view.h index 3d3cf5b6fed2..0e8e4fb090d8 100644 --- a/browser/ui/views/frame/brave_opaque_browser_frame_view.h +++ b/browser/ui/views/frame/brave_opaque_browser_frame_view.h @@ -25,6 +25,7 @@ class BraveOpaqueBrowserFrameView : public OpaqueBrowserFrameView { // OpaqueBrowserFrameView overrides: void OnPaint(gfx::Canvas* canvas) override; + int NonClientHitTest(const gfx::Point& point) override; private: std::unique_ptr frame_graphic_; From 5803d58c6748e715310e73f5a23356df25c858db Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Fri, 21 Oct 2022 20:32:56 +0900 Subject: [PATCH 03/12] Fix crash when using GTK theme When TabStrip::ShouldDrawColor() returns true, BrowserRootView will try to clip bounds tab strip. But When vertical tabstrip is enabled, converting coordinate system will fail as we have another widget for vertical tab strip. As we don't have to draw strokes below vertical tab strip, returning false will be okay for our purpose. --- browser/ui/views/tabs/brave_tab_strip.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/browser/ui/views/tabs/brave_tab_strip.cc b/browser/ui/views/tabs/brave_tab_strip.cc index 98b8aff971d2..74d4eb367772 100644 --- a/browser/ui/views/tabs/brave_tab_strip.cc +++ b/browser/ui/views/tabs/brave_tab_strip.cc @@ -25,6 +25,9 @@ BraveTabStrip::BraveTabStrip(std::unique_ptr controller) BraveTabStrip::~BraveTabStrip() = default; bool BraveTabStrip::ShouldDrawStrokes() const { + if (tabs::features::ShouldShowVerticalTabs()) + return false; + if (!TabStrip::ShouldDrawStrokes()) return false; From 2448aa40829bfa9326d69619b6cf0b5c7d9cc8c2 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Fri, 21 Oct 2022 21:31:47 +0900 Subject: [PATCH 04/12] Make frame views on linux use BraveOpaqueBrowserFrameView --- .../frame/brave_non_client_hit_test_helper.cc | 11 +++++------ .../frame/brave_non_client_hit_test_helper.h | 10 +++++----- .../views/frame/browser_frame_view_linux.cc | 14 ++++++++++++++ .../ui/views/frame/browser_frame_view_linux.h | 19 +++++++++++++++++++ 4 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.cc create mode 100644 chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper.cc b/browser/ui/views/frame/brave_non_client_hit_test_helper.cc index 57f7856a83fb..deddd55a8947 100644 --- a/browser/ui/views/frame/brave_non_client_hit_test_helper.cc +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper.cc @@ -1,14 +1,14 @@ /* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + * 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 http://mozilla.org/MPL/2.0/. */ #include "brave/browser/ui/views/frame/brave_non_client_hit_test_helper.h" #include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "ui/base/hit_test.h" #include "ui/views/window/hit_test_utils.h" -#include "chrome/browser/ui/views/toolbar/toolbar_view.h" namespace brave { @@ -19,5 +19,4 @@ int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point) { return views::GetHitTestComponent(browser_view->toolbar(), point); } -} // brave - +} // namespace brave diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper.h b/browser/ui/views/frame/brave_non_client_hit_test_helper.h index a6664244e459..fca90ac9f4b4 100644 --- a/browser/ui/views/frame/brave_non_client_hit_test_helper.h +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper.h @@ -1,14 +1,14 @@ /* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + * 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 http://mozilla.org/MPL/2.0/. */ #ifndef BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ #define BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ namespace gfx { class Point; -} // gfx +} // namespace gfx class BrowserView; @@ -19,6 +19,6 @@ namespace brave { // case, caller should depend on the default behavior. int NonClientHitTest(BrowserView* browser_view, const gfx::Point& point); -} // brave +} // namespace brave #endif // BRAVE_BROWSER_UI_VIEWS_FRAME_BRAVE_NON_CLIENT_HIT_TEST_HELPER_H_ diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.cc b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.cc new file mode 100644 index 000000000000..61728bbfa759 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.cc @@ -0,0 +1,14 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/ui/views/frame/browser_frame_view_linux.h" +#include "brave/browser/ui/views/frame/brave_opaque_browser_frame_view.h" + +#define OpaqueBrowserFrameView(frame, browser_view, layout) \ + BraveOpaqueBrowserFrameView(frame, browser_view, layout) + +#include "src/chrome/browser/ui/views/frame/browser_frame_view_linux.cc" + +#undef OpaqueBrowserFrameView diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h new file mode 100644 index 000000000000..ffdf8afac8d0 --- /dev/null +++ b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h @@ -0,0 +1,19 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_FRAME_VIEW_LINUX_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_FRAME_VIEW_LINUX_H_ + +#include "chrome/browser/ui/views/frame/opaque_browser_frame_view.h" + +// Make BrowserFrameViewLinux extends our BraveOpaqueBrowserFrameView +#include "brave/browser/ui/views/frame/brave_opaque_browser_frame_view.h" +#define OpaqueBrowserFrameView BraveOpaqueBrowserFrameView + +#include "src/chrome/browser/ui/views/frame/browser_frame_view_linux.h" + +#undef OpaqueBrowserFrameView + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_FRAME_VIEW_LINUX_H_ From 2ecad193eb43de5e37519c57e8f7bfb2d53501ae Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Fri, 21 Oct 2022 22:04:57 +0900 Subject: [PATCH 05/12] Clean up --- app/brave_generated_resources.grd | 4 ++-- browser/ui/views/frame/brave_browser_frame_mac.mm | 4 ++-- browser/ui/views/frame/brave_glass_browser_frame_view.cc | 3 +-- browser/ui/views/tabs/features.cc | 2 +- browser/ui/views/tabs/features.h | 4 ---- 5 files changed, 6 insertions(+), 11 deletions(-) diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index 745cc245f939..549d728fcad2 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -804,7 +804,7 @@ Or change later at $2brave://settings/ext Bookmark all tabs... - Show window title + Show title bar @@ -812,7 +812,7 @@ Or change later at $2brave://settings/ext Bookmark All Tabs... - Show Window Title + Show title bar diff --git a/browser/ui/views/frame/brave_browser_frame_mac.mm b/browser/ui/views/frame/brave_browser_frame_mac.mm index 02441143d930..51ff0580e729 100644 --- a/browser/ui/views/frame/brave_browser_frame_mac.mm +++ b/browser/ui/views/frame/brave_browser_frame_mac.mm @@ -18,8 +18,8 @@ void BraveBrowserFrameMac::GetWindowFrameTitlebarHeight( bool* override_titlebar_height, float* titlebar_height) { - // Don't override titlebar height if we supports vertical tab strip so that - // it can overlay our client view. The visibility of title bar will be + // Don't override titlebar height if the browser supports vertical tab strip + // so that it can overlay our client view. The visibility of title bar will be // controlled by BrowserNonClientFrameViewMac::UpdateWindowTitleVisibility. if (browser_->is_type_normal() && tabs::features::ShouldShowVerticalTabs()) return; diff --git a/browser/ui/views/frame/brave_glass_browser_frame_view.cc b/browser/ui/views/frame/brave_glass_browser_frame_view.cc index c55687cebe9e..591ee1502ba7 100644 --- a/browser/ui/views/frame/brave_glass_browser_frame_view.cc +++ b/browser/ui/views/frame/brave_glass_browser_frame_view.cc @@ -41,8 +41,7 @@ void BraveGlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) { } int BraveGlassBrowserFrameView::GetTopInset(bool restored) const { - if (browser_view()->browser()->is_type_normal() && - tabs::features::ShouldShowVerticalTabs()) { + if (tabs::features::ShouldShowVerticalTabs()) { if (!tabs::features::ShouldShowWindowTitleForVerticalTabs( browser_view()->browser())) return 0; diff --git a/browser/ui/views/tabs/features.cc b/browser/ui/views/tabs/features.cc index 7e819d77439e..6a48ff1c6696 100644 --- a/browser/ui/views/tabs/features.cc +++ b/browser/ui/views/tabs/features.cc @@ -72,7 +72,7 @@ std::pair GetLeadingTrailingCaptionButtonWidth( } // In this case, we use OpaqueBrowserFrameView. OpaqueBrowserFrameView has - // two types of frame button per platform but on Window, it uses image + // two types of frame button per platform but on Windows, it uses image // buttons. See OpaqueBrowserFrameView::GetFrameButtonStyle(). int width = 0; // Uses image icons diff --git a/browser/ui/views/tabs/features.h b/browser/ui/views/tabs/features.h index 41109debbe67..3d6348ad175b 100644 --- a/browser/ui/views/tabs/features.h +++ b/browser/ui/views/tabs/features.h @@ -14,10 +14,6 @@ namespace base { struct LOGICALLY_CONST Feature; } // namespace base -namespace views { -class View; -} // namespace views - class Browser; class BrowserFrame; From 0e8b9a6d22987ec5472c9c67f9b8d4348f767e80 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Sat, 22 Oct 2022 21:04:17 +0900 Subject: [PATCH 06/12] Make tests for title visibility and hit test --- browser/ui/browser_commands.cc | 8 ++ browser/ui/browser_commands.h | 2 + .../ui/views/frame/brave_browser_frame_mac.mm | 2 +- ..._non_client_hit_test_helper_browsertest.cc | 37 ++++++ .../tabs/brave_tab_context_menu_contents.cc | 15 +-- .../tabs/vertical_tab_strip_browsertest.cc | 111 ++++++++++++++++++ .../ui/views/frame/browser_frame_view_linux.h | 5 + .../ui/views/widget/native_widget_mac.h | 15 ++- .../ui/views/widget/native_widget_mac.mm | 6 + test/BUILD.gn | 2 + 10 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc create mode 100644 browser/ui/views/tabs/vertical_tab_strip_browsertest.cc diff --git a/browser/ui/browser_commands.cc b/browser/ui/browser_commands.cc index 81b1fd8fbb25..6130d2807c92 100644 --- a/browser/ui/browser_commands.cc +++ b/browser/ui/browser_commands.cc @@ -11,6 +11,7 @@ #include "brave/app/brave_command_ids.h" #include "brave/browser/debounce/debounce_service_factory.h" #include "brave/browser/net/brave_query_filter.h" +#include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/url_sanitizer/url_sanitizer_service_factory.h" #include "brave/components/brave_vpn/buildflags/buildflags.h" #include "brave/components/constants/pref_names.h" @@ -225,4 +226,11 @@ void CopyLinkWithStrictCleaning(Browser* browser, const GURL& url) { scw.WriteText(base::UTF8ToUTF16(final_url.spec())); } +void ToggleWindowTitleVisibilityForVerticalTabs(Browser* browser) { + auto* prefs = browser->profile()->GetOriginalProfile()->GetPrefs(); + prefs->SetBoolean( + brave_tabs::kVerticalTabsShowTitleOnWindow, + !prefs->GetBoolean(brave_tabs::kVerticalTabsShowTitleOnWindow)); +} + } // namespace brave diff --git a/browser/ui/browser_commands.h b/browser/ui/browser_commands.h index 2bc4dcd832df..24367061178c 100644 --- a/browser/ui/browser_commands.h +++ b/browser/ui/browser_commands.h @@ -31,6 +31,8 @@ void CopySanitizedURL(Browser* browser, const GURL& url); // - URLSanitizerService void CopyLinkWithStrictCleaning(Browser* browser, const GURL& url); +void ToggleWindowTitleVisibilityForVerticalTabs(Browser* browser); + } // namespace brave diff --git a/browser/ui/views/frame/brave_browser_frame_mac.mm b/browser/ui/views/frame/brave_browser_frame_mac.mm index 51ff0580e729..35170f56c9d9 100644 --- a/browser/ui/views/frame/brave_browser_frame_mac.mm +++ b/browser/ui/views/frame/brave_browser_frame_mac.mm @@ -18,7 +18,7 @@ void BraveBrowserFrameMac::GetWindowFrameTitlebarHeight( bool* override_titlebar_height, float* titlebar_height) { - // Don't override titlebar height if the browser supports vertical tab strip + // Don't override titlebar height if the browser supports vertical tab strip // so that it can overlay our client view. The visibility of title bar will be // controlled by BrowserNonClientFrameViewMac::UpdateWindowTitleVisibility. if (browser_->is_type_normal() && tabs::features::ShouldShowVerticalTabs()) diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc new file mode 100644 index 000000000000..e55b11a257fe --- /dev/null +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc @@ -0,0 +1,37 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/toolbar/reload_button.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "content/public/test/browser_test.h" +#include "ui/base/hit_test.h" + +using BraveNonClientHitTestHelperBrowserTest = InProcessBrowserTest; + +IN_PROC_BROWSER_TEST_F(BraveNonClientHitTestHelperBrowserTest, Toolbar) { + auto* browser_view = static_cast(browser()->window()); + auto* toolbar = browser_view->toolbar(); + auto* frame_view = browser_view->frame()->GetFrameView(); + + gfx::Point point; + views::View::ConvertPointToWidget(toolbar, &point); + + // Dragging a window with the toolbar on it should work. + EXPECT_EQ(HTCAPTION, frame_view->NonClientHitTest(point)); + + // It shouldn't be perceived as a HTCAPTION when it's not visible. + toolbar->SetVisible(false); + EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point)); + + // A coordinate on children of toolbar shouldn't be HTCAPTION so that users + // can interact with them. Checks a typical child of toolbar as a sanity + // check. + toolbar->SetVisible(true); + views::View::ConvertPointToWidget(toolbar->reload_button(), &point); + EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point)); +} diff --git a/browser/ui/views/tabs/brave_tab_context_menu_contents.cc b/browser/ui/views/tabs/brave_tab_context_menu_contents.cc index 65528c2e2b0c..12308d92f129 100644 --- a/browser/ui/views/tabs/brave_tab_context_menu_contents.cc +++ b/browser/ui/views/tabs/brave_tab_context_menu_contents.cc @@ -5,6 +5,7 @@ #include "brave/browser/ui/views/tabs/brave_tab_context_menu_contents.h" +#include "brave/browser/ui/browser_commands.h" #include "brave/browser/ui/tabs/brave_tab_menu_model.h" #include "brave/browser/ui/tabs/brave_tab_prefs.h" #include "brave/browser/ui/views/tabs/brave_browser_tab_strip_controller.h" @@ -51,11 +52,8 @@ void BraveTabContextMenuContents::RunMenuAt( bool BraveTabContextMenuContents::IsCommandIdChecked(int command_id) const { if (command_id == BraveTabMenuModel::CommandShowTitleBar) { - return controller_->browser() - ->profile() - ->GetOriginalProfile() - ->GetPrefs() - ->GetBoolean(brave_tabs::kVerticalTabsShowTitleOnWindow); + return tabs::features::ShouldShowWindowTitleForVerticalTabs( + controller_->browser()); } return false; @@ -135,11 +133,8 @@ void BraveTabContextMenuContents::ExecuteBraveCommand(int command_id) { chrome::BookmarkAllTabs(browser_); return; case BraveTabMenuModel::CommandShowTitleBar: { - auto* browser = controller_->browser(); - browser->profile()->GetOriginalProfile()->GetPrefs()->SetBoolean( - brave_tabs::kVerticalTabsShowTitleOnWindow, - !IsCommandIdChecked(command_id)); - BrowserView::GetBrowserViewForBrowser(browser)->InvalidateLayout(); + brave::ToggleWindowTitleVisibilityForVerticalTabs(browser_); + BrowserView::GetBrowserViewForBrowser(browser_)->InvalidateLayout(); return; } default: diff --git a/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc new file mode 100644 index 000000000000..3b4956b67908 --- /dev/null +++ b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc @@ -0,0 +1,111 @@ +/* Copyright (c) 2022 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 http://mozilla.org/MPL/2.0/. */ + +#include "base/test/scoped_feature_list.h" +#include "brave/browser/ui/browser_commands.h" +#include "brave/browser/ui/views/tabs/features.h" +#include "build/build_config.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h" +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "content/public/test/browser_test.h" + +#if BUILDFLAG(IS_WIN) +#include "chrome/browser/ui/views/frame/glass_browser_frame_view.h" +#endif + +#if BUILDFLAG(IS_MAC) +#include "ui/views/widget/native_widget_mac.h" +#endif + +#if defined(USE_AURA) +#include "chrome/browser/ui/views/frame/opaque_browser_frame_view.h" +#endif + +class VerticalTabStripBrowserTest : public InProcessBrowserTest { + public: + VerticalTabStripBrowserTest() + : feature_list_(tabs::features::kBraveVerticalTabs) {} + + ~VerticalTabStripBrowserTest() override = default; + + const BrowserView* browser_view() const { + return static_cast(browser()->window()); + } + BrowserView* browser_view() { + return static_cast(browser()->window()); + } + BrowserNonClientFrameView* browser_non_client_frame_view() { + return browser_view()->frame()->GetFrameView(); + } + const BrowserNonClientFrameView* browser_non_client_frame_view() const { + return browser_view()->frame()->GetFrameView(); + } + + // Returns the visibility of window title is actually changed by a frame or + // a widget. If we can't access to actual title view, returns a value which + // window title will be synchronized to. + bool IsWindowTitleViewVisible() const { +#if BUILDFLAG(IS_MAC) + auto* native_widget = static_cast( + browser_view()->GetWidget()->native_widget_private()); + if (!native_widget->has_overridden_window_title_visibility()) { + // Returns default visibility + return browser_view() + ->GetWidget() + ->widget_delegate() + ->ShouldShowWindowTitle(); + } + + return native_widget->overridden_window_title_visibility(); +#elif BUILDFLAG(IS_WIN) + if (browser_view()->GetWidget()->ShouldUseNativeFrame()) { + return static_cast( + browser_non_client_frame_view()) + ->window_title_for_testing() + ->GetVisible(); + } +#endif + +#if defined(USE_AURA) + return static_cast( + browser_non_client_frame_view()) + ->ShouldShowWindowTitle(); +#endif + } + + base::test::ScopedFeatureList feature_list_; +}; + +IN_PROC_BROWSER_TEST_F(VerticalTabStripBrowserTest, WindowTitle) { + // Pre-condition: Window title is "visible" by default on vertical tabs + ASSERT_TRUE(tabs::features::ShouldShowVerticalTabs()); + ASSERT_TRUE(tabs::features::ShouldShowWindowTitleForVerticalTabs(browser())); + ASSERT_TRUE(browser_view()->ShouldShowWindowTitle()); + ASSERT_TRUE(IsWindowTitleViewVisible()); + + // Hide window title bar + brave::ToggleWindowTitleVisibilityForVerticalTabs(browser()); + browser_non_client_frame_view()->Layout(); + EXPECT_FALSE(tabs::features::ShouldShowWindowTitleForVerticalTabs(browser())); + EXPECT_FALSE(browser_view()->ShouldShowWindowTitle()); +#if !BUILDFLAG(IS_LINUX) + // TODO(sko) For now, we can't hide window title bar entirely on Linux. + // We're using a minimum height for it. + EXPECT_EQ(0, + browser_non_client_frame_view()->GetTopInset(/*restored=*/false)); +#endif + EXPECT_FALSE(IsWindowTitleViewVisible()); + + // Show window title bar + brave::ToggleWindowTitleVisibilityForVerticalTabs(browser()); + browser_non_client_frame_view()->Layout(); + EXPECT_TRUE(tabs::features::ShouldShowWindowTitleForVerticalTabs(browser())); + EXPECT_TRUE(browser_view()->ShouldShowWindowTitle()); + EXPECT_GE(browser_non_client_frame_view()->GetTopInset(/*restored=*/false), + 0); + EXPECT_TRUE(IsWindowTitleViewVisible()); +} diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h index ffdf8afac8d0..8d4d087bcbc0 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h +++ b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_linux.h @@ -16,4 +16,9 @@ #undef OpaqueBrowserFrameView +// Sanity check at compile time. +static_assert( + std::is_base_of_v, + "BrowserFrameViewLinux should be a child of BraveOpaqueBrowserFrameView"); + #endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_FRAME_VIEW_LINUX_H_ diff --git a/chromium_src/ui/views/widget/native_widget_mac.h b/chromium_src/ui/views/widget/native_widget_mac.h index 7b592a5d3d09..9198214b7241 100644 --- a/chromium_src/ui/views/widget/native_widget_mac.h +++ b/chromium_src/ui/views/widget/native_widget_mac.h @@ -8,8 +8,19 @@ #include "ui/views/widget/native_widget_private.h" -#define SetWindowIcons \ - SetWindowTitleVisibility(bool visible); \ +#define SetWindowIcons \ + SetWindowTitleVisibility(bool visible); \ + bool has_overridden_window_title_visibility() const { \ + return overridden_window_title_visibility_.has_value(); \ + } \ + bool overridden_window_title_visibility() const { \ + return overridden_window_title_visibility_.value(); \ + } \ + \ + private: \ + absl::optional overridden_window_title_visibility_; \ + \ + public: \ void SetWindowIcons #include "src/ui/views/widget/native_widget_mac.h" diff --git a/chromium_src/ui/views/widget/native_widget_mac.mm b/chromium_src/ui/views/widget/native_widget_mac.mm index 0477cd65b6c3..4e4b668679bd 100644 --- a/chromium_src/ui/views/widget/native_widget_mac.mm +++ b/chromium_src/ui/views/widget/native_widget_mac.mm @@ -8,7 +8,13 @@ namespace views { void NativeWidgetMac::SetWindowTitleVisibility(bool visible) { + if (overridden_window_title_visibility_.has_value() && + visible == *overridden_window_title_visibility_) { + return; + } + GetNSWindowMojo()->SetWindowTitleVisibility(visible); + overridden_window_title_visibility_ = visible; } } // namespace views diff --git a/test/BUILD.gn b/test/BUILD.gn index b08678970722..509726b4d2bd 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1067,11 +1067,13 @@ test("brave_browser_tests") { "//brave/browser/ui/views/brave_actions/brave_actions_container_browsertest.cc", "//brave/browser/ui/views/brave_ads/notification_ad_popup_browsertest.cc", "//brave/browser/ui/views/brave_shields/cookie_list_opt_in_browsertest.cc", + "//brave/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc", "//brave/browser/ui/views/frame/brave_tabs_search_button_browsertest.cc", "//brave/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc", "//brave/browser/ui/views/omnibox/omnibox_autocomplete_browsertest.cc", "//brave/browser/ui/views/profiles/brave_profile_menu_view_browsertest.cc", "//brave/browser/ui/views/tabs/brave_tab_context_menu_contents_browsertest.cc", + "//brave/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc", "//brave/browser/ui/views/toolbar/brave_toolbar_view_browsertest.cc", "//brave/browser/ui/views/toolbar/wallet_button_browsertest.cc", ] From 08f1c1628cc417b0142c27b6fa406160f31f16d9 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Sun, 23 Oct 2022 16:06:13 +0900 Subject: [PATCH 07/12] Update BrowserFrameViewLayoutLinux::NonClientTopHeight --- .../frame/browser_frame_view_layout_linux.cc | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc index 3b67f22ea3d4..b401c437974d 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc +++ b/chromium_src/chrome/browser/ui/views/frame/browser_frame_view_layout_linux.cc @@ -25,19 +25,24 @@ int BrowserFrameViewLayoutLinux::NonClientTopHeight(bool restored) const { return OpaqueBrowserFrameViewLayout::NonClientTopHeight(restored); } - if (tabs::features::ShouldShowWindowTitleForVerticalTabs( - view_->browser_view()->browser())) { - return 30; - } - // TODO(sko) For now, I couldn't find a way to overlay caption buttons // on toolbar. Once it gets possible, we shouldn't reserve non client top - // height. - if (view_->ShouldShowCaptionButtons()) { - // Uses the same caption height defined in - // c/b/u/v/frame/opaque_browser_frame_view_layout.cc - return 18; - } + // height when window title isn't visible. + + // Adding 2px of vertical padding puts at least 1 px of space on the top and + // bottom of the element. + constexpr int kVerticalPadding = 2; + // delegate_->GetIconSize() also considers the default font's height so + // title will be visible. + const int icon_height = FrameEdgeInsets(restored).top() + + delegate_->GetIconSize() + kVerticalPadding; + + const int caption_button_height = + DefaultCaptionButtonY(restored) + + 18 /*kCaptionButtonHeight in OpaqueBrowserFrameView*/ + + kCaptionButtonBottomPadding; + return std::max(icon_height, caption_button_height) + + kCaptionButtonBottomPadding; } return OpaqueBrowserFrameViewLayout::NonClientTopHeight(restored); From 059a8c76bd970711b1584889a2aba8ccc6d78509 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Mon, 24 Oct 2022 08:34:53 +0900 Subject: [PATCH 08/12] Add DCHECK --- browser/ui/views/tabs/vertical_tab_strip_browsertest.cc | 2 +- chromium_src/ui/views/widget/native_widget_mac.h | 4 +--- chromium_src/ui/views/widget/native_widget_mac.mm | 7 +++++++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc index 3b4956b67908..88a49933eade 100644 --- a/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc +++ b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc @@ -60,7 +60,7 @@ class VerticalTabStripBrowserTest : public InProcessBrowserTest { ->ShouldShowWindowTitle(); } - return native_widget->overridden_window_title_visibility(); + return native_widget->GetOverriddenWindowTitleVisibility(); #elif BUILDFLAG(IS_WIN) if (browser_view()->GetWidget()->ShouldUseNativeFrame()) { return static_cast( diff --git a/chromium_src/ui/views/widget/native_widget_mac.h b/chromium_src/ui/views/widget/native_widget_mac.h index 9198214b7241..b04733190615 100644 --- a/chromium_src/ui/views/widget/native_widget_mac.h +++ b/chromium_src/ui/views/widget/native_widget_mac.h @@ -13,9 +13,7 @@ bool has_overridden_window_title_visibility() const { \ return overridden_window_title_visibility_.has_value(); \ } \ - bool overridden_window_title_visibility() const { \ - return overridden_window_title_visibility_.value(); \ - } \ + bool GetOverriddenWindowTitleVisibility() const; \ \ private: \ absl::optional overridden_window_title_visibility_; \ diff --git a/chromium_src/ui/views/widget/native_widget_mac.mm b/chromium_src/ui/views/widget/native_widget_mac.mm index 4e4b668679bd..d6e2c5ec29eb 100644 --- a/chromium_src/ui/views/widget/native_widget_mac.mm +++ b/chromium_src/ui/views/widget/native_widget_mac.mm @@ -17,4 +17,11 @@ overridden_window_title_visibility_ = visible; } +bool NativeWidgetMac::GetOverriddenWindowTitleVisibility() const { + DCHECK(has_overridden_window_title_visibility()) + << "Didn't call SetWindowTitleVisibility(). Use " + "WidgetDelegate::ShouldShowWindowTitle() instead."; + return *overridden_window_title_visibility_; +} + } // namespace views From 2ff2d399246808350d84dcafc3232ce4e504c5b4 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Mon, 24 Oct 2022 16:00:43 +0900 Subject: [PATCH 09/12] Fix test failure --- .../views/frame/brave_non_client_hit_test_helper_browsertest.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc index e55b11a257fe..711bf937cfa3 100644 --- a/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc @@ -32,6 +32,7 @@ IN_PROC_BROWSER_TEST_F(BraveNonClientHitTestHelperBrowserTest, Toolbar) { // can interact with them. Checks a typical child of toolbar as a sanity // check. toolbar->SetVisible(true); + point = {}; views::View::ConvertPointToWidget(toolbar->reload_button(), &point); EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point)); } From 853171fe2d5bf2e633d5e7475a59b6bdc3c5f0c7 Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Mon, 24 Oct 2022 21:19:32 +0900 Subject: [PATCH 10/12] Fix include bug --- ...browser_non_client_frame_view_factory_views.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_factory_views.cc b/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_factory_views.cc index 5b4449f063e7..f42435020d29 100644 --- a/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_factory_views.cc +++ b/chromium_src/chrome/browser/ui/views/frame/browser_non_client_frame_view_factory_views.cc @@ -3,6 +3,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include +#include + #include "build/build_config.h" #if BUILDFLAG(IS_WIN) @@ -10,6 +13,10 @@ #define GlassBrowserFrameView BraveGlassBrowserFrameView #endif +// This file is included for all platform by upstream source code and we need +// to include this first to avoid #define conflicts. +#include "chrome/browser/ui/views/frame/browser_frame_view_linux.h" + #include "brave/browser/ui/views/frame/brave_opaque_browser_frame_view.h" #define OpaqueBrowserFrameView BraveOpaqueBrowserFrameView @@ -17,6 +24,14 @@ #undef OpaqueBrowserFrameView +// A sanity check for our macro +static_assert( + std::is_same_v, + decltype(std::function{ + chrome::CreateOpaqueBrowserFrameView})::result_type>, + "CreateOpaqueBrowserFrameView is not returning " + "BraveOpaqueBrowserFrameView"); + #if BUILDFLAG(IS_WIN) #undef GlassBrowserFrameView #endif From b0577ba5143f8a0da947b09c3224a38c5ef214ad Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Tue, 25 Oct 2022 11:23:14 +0900 Subject: [PATCH 11/12] Fix test failure on Windows --- .../views/frame/brave_non_client_hit_test_helper_browsertest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc index 711bf937cfa3..35c1020732b5 100644 --- a/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc +++ b/browser/ui/views/frame/brave_non_client_hit_test_helper_browsertest.cc @@ -32,7 +32,7 @@ IN_PROC_BROWSER_TEST_F(BraveNonClientHitTestHelperBrowserTest, Toolbar) { // can interact with them. Checks a typical child of toolbar as a sanity // check. toolbar->SetVisible(true); - point = {}; + point = gfx::Point(); views::View::ConvertPointToWidget(toolbar->reload_button(), &point); EXPECT_NE(HTCAPTION, frame_view->NonClientHitTest(point)); } From 8ec1ab6f8fd6646e1393088f1ba9228cafc92e4e Mon Sep 17 00:00:00 2001 From: "sangwoo.ko" Date: Tue, 25 Oct 2022 11:30:28 +0900 Subject: [PATCH 12/12] Fix linux test case --- browser/ui/views/tabs/vertical_tab_strip_browsertest.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc index 88a49933eade..eb18e5be45e8 100644 --- a/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc +++ b/browser/ui/views/tabs/vertical_tab_strip_browsertest.cc @@ -17,6 +17,10 @@ #include "chrome/browser/ui/views/frame/glass_browser_frame_view.h" #endif +#if BUILDFLAG(IS_LINUX) +#include "chrome/common/pref_names.h" +#endif + #if BUILDFLAG(IS_MAC) #include "ui/views/widget/native_widget_mac.h" #endif @@ -81,6 +85,10 @@ class VerticalTabStripBrowserTest : public InProcessBrowserTest { }; IN_PROC_BROWSER_TEST_F(VerticalTabStripBrowserTest, WindowTitle) { +#if BUILDFLAG(IS_LINUX) + browser()->profile()->GetPrefs()->SetBoolean(prefs::kUseCustomChromeFrame, + true); +#endif // Pre-condition: Window title is "visible" by default on vertical tabs ASSERT_TRUE(tabs::features::ShouldShowVerticalTabs()); ASSERT_TRUE(tabs::features::ShouldShowWindowTitleForVerticalTabs(browser()));