From 7d3fc7cf971134ccf452a7ebfefb623ce5def479 Mon Sep 17 00:00:00 2001 From: Sangwoo Ko Date: Fri, 3 Apr 2026 10:13:52 +0900 Subject: [PATCH] [Containers] Activate PageActionIconView for Partitioned Storage on right click (#35194) Activate PageActionIconView for Partitioned Storage on right click As requested by the design team, we activate the PageActionIconView for the Partitioned Storage on right click. --- browser/ui/brave_browser_actions.cc | 19 ++++++++++--- browser/ui/browser_commands.cc | 6 +++++ browser/ui/browser_commands.h | 4 +++ .../page_action/page_action_view_unittest.cc | 27 +++++++++++++++++++ ...titioned_storage_page_action_controller.cc | 5 ++++ .../page_action/page_action_controller.cc | 7 +++++ .../ui/views/page_action/page_action_model.cc | 14 ++++++++++ .../ui/views/page_action/page_action_view.cc | 16 +++++++++++ .../ui/views/page_action/page_action_view.h | 7 +++++ .../mock_page_action_controller.h | 4 +++ .../test_support/mock_page_action_model.h | 8 ++++++ ...page_action-page_action_controller.h.patch | 13 ++++++--- ...iews-page_action-page_action_model.h.patch | 16 ++++++++--- .../page_action/page_action_controller.h.toml | 11 ++++++-- .../page_action/page_action_model.h.toml | 12 +++++++-- 15 files changed, 154 insertions(+), 15 deletions(-) diff --git a/browser/ui/brave_browser_actions.cc b/browser/ui/brave_browser_actions.cc index 1bad07d841a..cdcc0de7855 100644 --- a/browser/ui/brave_browser_actions.cc +++ b/browser/ui/brave_browser_actions.cc @@ -7,6 +7,7 @@ #include "base/functional/callback_helpers.h" #include "base/types/to_address.h" +#include "brave/browser/ui/browser_commands.h" #include "brave/components/ai_chat/core/common/buildflags/buildflags.h" #include "brave/components/containers/buildflags/buildflags.h" #include "brave/components/playlist/core/common/features.h" @@ -81,10 +82,20 @@ void BraveBrowserActions::InitializeBrowserActions() { #if BUILDFLAG(ENABLE_CONTAINERS) if (base::FeatureList::IsEnabled(containers::features::kContainers)) { - root_action_item_->AddChild(actions::ActionItem::Builder(base::DoNothing()) - .SetActionId(kActionShowPartitionedStorage) - .SetEnabled(true) - .Build()); + root_action_item_->AddChild( + actions::ActionItem::Builder( + // Safe to bind bwi to the callback because root_action_item_ is + // going to be destroyed on the base class's destructor while the + // browser window interface is also member of the base class. + base::BindRepeating( + [](BrowserWindowInterface* bwi, actions::ActionItem* item, + actions::ActionInvocationContext context) { + brave::OpenContainerMenuOnPageActionView(bwi); + }, + bwi)) + .SetActionId(kActionShowPartitionedStorage) + .SetEnabled(true) + .Build()); } #endif } diff --git a/browser/ui/browser_commands.cc b/browser/ui/browser_commands.cc index 7f9916b7224..bf03390ccac 100644 --- a/browser/ui/browser_commands.cc +++ b/browser/ui/browser_commands.cc @@ -21,6 +21,7 @@ #include "base/i18n/time_formatting.h" #include "base/logging.h" #include "base/memory/scoped_refptr.h" +#include "base/notimplemented.h" #include "base/notreached.h" #include "base/path_service.h" #include "base/strings/utf_string_conversions.h" @@ -1195,6 +1196,11 @@ void OpenUrlInContainer(BrowserWindowInterface* browser_window, Navigate(¶ms); } + +void OpenContainerMenuOnPageActionView(BrowserWindowInterface* browser_window) { + // TODO(https://github.com/brave/brave-browser/issues/53350) + NOTIMPLEMENTED(); +} #endif } // namespace brave diff --git a/browser/ui/browser_commands.h b/browser/ui/browser_commands.h index b8471b5d05c..572ecd5fc3f 100644 --- a/browser/ui/browser_commands.h +++ b/browser/ui/browser_commands.h @@ -163,6 +163,10 @@ void OpenTabUrlInContainer(BrowserWindowInterface* browser_window, void OpenUrlInContainer(BrowserWindowInterface* browser_window, const GURL& url, const containers::mojom::ContainerPtr& container); + +// Opens the container menu on the page action view if the active tab is in a +// container. +void OpenContainerMenuOnPageActionView(BrowserWindowInterface* browser); #endif } // namespace brave diff --git a/browser/ui/views/page_action/page_action_view_unittest.cc b/browser/ui/views/page_action/page_action_view_unittest.cc index c5f1de40c44..0957bfb07e9 100644 --- a/browser/ui/views/page_action/page_action_view_unittest.cc +++ b/browser/ui/views/page_action/page_action_view_unittest.cc @@ -11,6 +11,7 @@ #include "chrome/test/views/chrome_views_test_base.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "ui/events/event.h" #include "ui/views/style/platform_style.h" #include "ui/views/test/views_test_utils.h" #include "ui/views/view_class_properties.h" @@ -188,4 +189,30 @@ TEST_F(PageActionViewTest, OverrideHeightIgnoreSizeBounds) { views::LayoutAlignment::kCenter); } +TEST_F(PageActionViewTest, OverrideTriggerableEventUsesCallback) { + EXPECT_CALL(*model(), GetOverrideTriggerableEvent()) + .WillRepeatedly(Return(ui::EF_RIGHT_MOUSE_BUTTON)); + + ui::MouseEvent left_press(ui::EventType::kMousePressed, gfx::Point(5, 5), + gfx::Point(5, 5), base::TimeTicks::Now(), + ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON); + ui::MouseEvent right_press(ui::EventType::kMousePressed, gfx::Point(5, 5), + gfx::Point(5, 5), base::TimeTicks::Now(), + ui::EF_RIGHT_MOUSE_BUTTON, + ui::EF_RIGHT_MOUSE_BUTTON); + page_action_view()->OnPageActionModelChanged(*model()); + + EXPECT_FALSE(page_action_view()->IsTriggerableEvent(left_press)); + EXPECT_TRUE(page_action_view()->IsTriggerableEvent(right_press)); + testing::Mock::VerifyAndClearExpectations(model()); + + EXPECT_CALL(*model(), GetOverrideTriggerableEvent()) + .WillRepeatedly(Return(std::nullopt)); + page_action_view()->OnPageActionModelChanged(*model()); + + EXPECT_TRUE(page_action_view()->IsTriggerableEvent(left_press)); + EXPECT_FALSE(page_action_view()->IsTriggerableEvent(right_press)); + testing::Mock::VerifyAndClearExpectations(model()); +} + } // namespace page_actions diff --git a/browser/ui/views/page_action/partitioned_storage_page_action_controller.cc b/browser/ui/views/page_action/partitioned_storage_page_action_controller.cc index 278dc883c42..6cef4586d18 100644 --- a/browser/ui/views/page_action/partitioned_storage_page_action_controller.cc +++ b/browser/ui/views/page_action/partitioned_storage_page_action_controller.cc @@ -16,6 +16,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/actions/chrome_action_id.h" #include "content/public/browser/web_contents.h" +#include "ui/events/event_constants.h" #include "ui/gfx/text_elider.h" namespace page_actions { @@ -84,6 +85,8 @@ void PartitionedStoragePageActionController::UpdatePageAction() { page_action_controller_->ClearOverrideChipColors( kActionShowPartitionedStorage); page_action_controller_->ClearOverrideHeight(kActionShowPartitionedStorage); + page_action_controller_->SetOverrideTriggerableEvent( + kActionShowPartitionedStorage, std::nullopt); return; } @@ -109,6 +112,8 @@ void PartitionedStoragePageActionController::UpdatePageAction() { page_action_controller_->OverrideChipColors( kActionShowPartitionedStorage, model->background_color(), SK_ColorWHITE); page_action_controller_->SetOverrideHeight(kActionShowPartitionedStorage, 20); + page_action_controller_->SetOverrideTriggerableEvent( + kActionShowPartitionedStorage, ui::EF_RIGHT_MOUSE_BUTTON); } } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/views/page_action/page_action_controller.cc b/chromium_src/chrome/browser/ui/views/page_action/page_action_controller.cc index 32e33bbf4d8..71ee88cfd22 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/page_action_controller.cc +++ b/chromium_src/chrome/browser/ui/views/page_action/page_action_controller.cc @@ -38,4 +38,11 @@ void PageActionControllerImpl::ClearOverrideHeight( FindPageActionModel(action_id).SetOverrideHeight(PassKey(), std::nullopt); } +void PageActionControllerImpl::SetOverrideTriggerableEvent( + actions::ActionId action_id, + std::optional event_flags) { + FindPageActionModel(action_id).SetOverrideTriggerableEvent(PassKey(), + event_flags); +} + } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/views/page_action/page_action_model.cc b/chromium_src/chrome/browser/ui/views/page_action/page_action_model.cc index 1b547d8df5b..1f6664d80fd 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/page_action_model.cc +++ b/chromium_src/chrome/browser/ui/views/page_action/page_action_model.cc @@ -54,4 +54,18 @@ std::optional PageActionModel::GetOverrideHeight() const { return override_height_; } +void PageActionModel::SetOverrideTriggerableEvent( + base::PassKey, + std::optional event_flags) { + if (override_triggerable_event_flags_ == event_flags) { + return; + } + override_triggerable_event_flags_ = event_flags; + NotifyChange(Property::kOverrideTriggerableEvent); +} + +std::optional PageActionModel::GetOverrideTriggerableEvent() const { + return override_triggerable_event_flags_; +} + } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/views/page_action/page_action_view.cc b/chromium_src/chrome/browser/ui/views/page_action/page_action_view.cc index 00edf19466a..58acd7bebb8 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/page_action_view.cc +++ b/chromium_src/chrome/browser/ui/views/page_action/page_action_view.cc @@ -8,6 +8,7 @@ #include #include "chrome/browser/ui/views/page_action/page_action_model.h" +#include "ui/events/event_constants.h" #include "ui/gfx/geometry/rect.h" #include "ui/views/layout/layout_types.h" #include "ui/views/layout/proposed_layout.h" @@ -16,9 +17,11 @@ #define GetMinimumSize GetMinimumSize_Chromium #define OnNewActiveController OnNewActiveController_Chromium +#define OnPageActionModelChanged OnPageActionModelChanged_Chromium #include +#undef OnPageActionModelChanged #undef OnNewActiveController #undef GetMinimumSize @@ -135,4 +138,17 @@ void PageActionView::OnNewActiveController(PageActionController* controller) { OnPageActionModelVisualRefresh(observation_.GetSource()); } +void PageActionView::OnPageActionModelChanged( + const PageActionModelInterface& model) { + PageActionView::OnPageActionModelChanged_Chromium(model); + + const PageActionModelInterface* source = observation_.GetSource(); + if (source) { + // ui::EF_LEFT_MOUSE_BUTTON is the default triggerable event flags of Button + // class. + SetTriggerableEventFlags(source->GetOverrideTriggerableEvent().value_or( + ui::EF_LEFT_MOUSE_BUTTON)); + } +} + } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/views/page_action/page_action_view.h b/chromium_src/chrome/browser/ui/views/page_action/page_action_view.h index d49cbdb8fd4..0a76e4230a4 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/page_action_view.h +++ b/chromium_src/chrome/browser/ui/views/page_action/page_action_view.h @@ -7,6 +7,7 @@ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_VIEW_H_ #include "chrome/browser/ui/views/location_bar/icon_label_bubble_view.h" +#include "chrome/browser/ui/views/page_action/page_action_model_observer.h" #include "ui/views/view.h" #include "ui/views/widget/native_widget_delegate.h" @@ -26,6 +27,11 @@ void OnPageActionModelVisualRefresh(PageActionModelInterface* model); \ bool ShouldAlwaysShowLabel() +// Make a OnPageActionModelChanged wrapper +#define OnPageActionModelChanged(...) \ + OnPageActionModelChanged_Chromium(__VA_ARGS__); \ + void OnPageActionModelChanged(__VA_ARGS__) + // Make a GetMinimumSize wrapper #define GetMinimumSize() \ GetMinimumSize_Chromium() const; \ @@ -40,6 +46,7 @@ #undef OnNewActiveController #undef GetMinimumSize +#undef OnPageActionModelChanged #undef ShouldShowLabelAfterAnimation #endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_PAGE_ACTION_PAGE_ACTION_VIEW_H_ diff --git a/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_controller.h b/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_controller.h index 57b067f7f7b..fdc84fbd0e1 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_controller.h +++ b/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_controller.h @@ -39,6 +39,10 @@ class MockPageActionController : public MockPageActionController_Chromium { ClearOverrideHeight, (actions::ActionId action_id), (override)); + MOCK_METHOD(void, + SetOverrideTriggerableEvent, + (actions::ActionId action_id, std::optional event_flags), + (override)); }; } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_model.h b/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_model.h index 0ba0becf804..211f5d35577 100644 --- a/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_model.h +++ b/chromium_src/chrome/browser/ui/views/page_action/test_support/mock_page_action_model.h @@ -41,6 +41,14 @@ class MockPageActionModel : public MockPageActionModel_Chromium { (base::PassKey, std::optional), (override)); MOCK_METHOD(std::optional, GetOverrideHeight, (), (const, override)); + MOCK_METHOD(void, + SetOverrideTriggerableEvent, + (base::PassKey, std::optional), + (override)); + MOCK_METHOD(std::optional, + GetOverrideTriggerableEvent, + (), + (const, override)); }; } // namespace page_actions diff --git a/patches/chrome-browser-ui-views-page_action-page_action_controller.h.patch b/patches/chrome-browser-ui-views-page_action-page_action_controller.h.patch index 9099cfc7077..3e20147b146 100644 --- a/patches/chrome-browser-ui-views-page_action-page_action_controller.h.patch +++ b/patches/chrome-browser-ui-views-page_action-page_action_controller.h.patch @@ -1,8 +1,8 @@ diff --git a/chrome/browser/ui/views/page_action/page_action_controller.h b/chrome/browser/ui/views/page_action/page_action_controller.h -index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565b05e4efd 100644 +index f8b60561923541d940cbc6c7fdee6fa0d025584f..69bd78e2632769ee82bf473c668576c660109869 100644 --- a/chrome/browser/ui/views/page_action/page_action_controller.h +++ b/chrome/browser/ui/views/page_action/page_action_controller.h -@@ -109,6 +109,22 @@ class PageActionController { +@@ -109,6 +109,27 @@ class PageActionController { public: virtual ~PageActionController() = default; @@ -21,11 +21,16 @@ index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565 + virtual void SetOverrideHeight(actions::ActionId action_id, + int height) = 0; + virtual void ClearOverrideHeight(actions::ActionId action_id) = 0; ++ ++ // When set, `PageActionView::IsTriggerableEvent` uses this callback instead of ++ // the default logic. Pass nullopt to clear. ++ virtual void SetOverrideTriggerableEvent(actions::ActionId action_id, ++ std::optional event_flags) = 0; + // Requests that the page action be shown or hidden. virtual void Show(actions::ActionId action_id) = 0; virtual void Hide(actions::ActionId action_id) = 0; -@@ -246,6 +262,16 @@ class PageActionControllerImpl : public PageActionController, +@@ -246,6 +267,18 @@ class PageActionControllerImpl : public PageActionController, PageActionControllerImpl& operator=(const PageActionControllerImpl&) = delete; ~PageActionControllerImpl() override; @@ -38,6 +43,8 @@ index f8b60561923541d940cbc6c7fdee6fa0d025584f..04e8d51f72cbd59f23ba1b706fa27565 + void SetOverrideHeight(actions::ActionId action_id, + int height) override; + void ClearOverrideHeight(actions::ActionId action_id) override; ++ void SetOverrideTriggerableEvent(actions::ActionId action_id, ++ std::optional event_flags) override; + void Initialize( tabs::TabInterface& tab_interface, diff --git a/patches/chrome-browser-ui-views-page_action-page_action_model.h.patch b/patches/chrome-browser-ui-views-page_action-page_action_model.h.patch index 66b97bbef21..90a184ae1d2 100644 --- a/patches/chrome-browser-ui-views-page_action-page_action_model.h.patch +++ b/patches/chrome-browser-ui-views-page_action-page_action_model.h.patch @@ -1,8 +1,8 @@ diff --git a/chrome/browser/ui/views/page_action/page_action_model.h b/chrome/browser/ui/views/page_action/page_action_model.h -index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd1a491c28 100644 +index a3d1abf58e71b25edb1487e2cf92c5587539d85b..e928a6a2c80bac025e6a64258fdc3c693e08562e 100644 --- a/chrome/browser/ui/views/page_action/page_action_model.h +++ b/chrome/browser/ui/views/page_action/page_action_model.h -@@ -33,6 +33,19 @@ class PageActionModelInterface { +@@ -33,6 +33,22 @@ class PageActionModelInterface { public: PageActionModelInterface() = default; virtual ~PageActionModelInterface() = default; @@ -14,15 +14,18 @@ index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd + bool always_show) = 0; + virtual void SetOverrideHeight(base::PassKey, + std::optional height) = 0; ++ virtual void SetOverrideTriggerableEvent(base::PassKey, ++ std::optional event_flags) = 0; + + virtual std::optional GetOverrideBackgroundColor() const = 0; + virtual std::optional GetOverrideForegroundColor() const = 0; + virtual bool GetAlwaysShowLabel() const = 0; + virtual std::optional GetOverrideHeight() const = 0; ++ virtual std::optional GetOverrideTriggerableEvent() const = 0; virtual void AddObserver(PageActionModelObserver* observer) = 0; virtual void RemoveObserver(PageActionModelObserver* observer) = 0; -@@ -120,6 +133,27 @@ class PageActionModel : public PageActionModelInterface { +@@ -120,6 +136,31 @@ class PageActionModel : public PageActionModelInterface { PageActionModel& operator=(const PageActionModel&) = delete; ~PageActionModel() override; @@ -34,29 +37,34 @@ index a3d1abf58e71b25edb1487e2cf92c5587539d85b..daffa064f8e97b9ba1e6f0100dfd70cd + bool always_show) override; + void SetOverrideHeight(base::PassKey, + std::optional height_px) override; ++ void SetOverrideTriggerableEvent(base::PassKey, ++ std::optional event_flags) override; + std::optional GetOverrideBackgroundColor() const override; + std::optional GetOverrideForegroundColor() const override; + bool GetAlwaysShowLabel() const override; + std::optional GetOverrideHeight() const override; ++ std::optional GetOverrideTriggerableEvent() const override; + + private: + std::optional override_background_color_; + std::optional override_foreground_color_; + bool always_show_label_ = false; + std::optional override_height_; ++ std::optional override_triggerable_event_flags_; + + public: + void AddObserver(PageActionModelObserver* observer) override; void RemoveObserver(PageActionModelObserver* observer) override; -@@ -210,6 +244,9 @@ class PageActionModel : public PageActionModelInterface { +@@ -210,6 +251,10 @@ class PageActionModel : public PageActionModelInterface { // per-property reentrancy checks. enum class Property { kShowRequested, + kAlwaysShowLabel, + kOverrideChipColors, + kOverrideHeight, ++ kOverrideTriggerableEvent, kShouldShowSuggestionChip, kSuggestionChipConfig, kTabActive, diff --git a/rewrite/chrome/browser/ui/views/page_action/page_action_controller.h.toml b/rewrite/chrome/browser/ui/views/page_action/page_action_controller.h.toml index a61a9d0f690..ee1198d2e42 100644 --- a/rewrite/chrome/browser/ui/views/page_action/page_action_controller.h.toml +++ b/rewrite/chrome/browser/ui/views/page_action/page_action_controller.h.toml @@ -22,7 +22,12 @@ replace = '''\1 // Override the height of the PageActionView virtual void SetOverrideHeight(actions::ActionId action_id, int height) = 0; - virtual void ClearOverrideHeight(actions::ActionId action_id) = 0;''' + virtual void ClearOverrideHeight(actions::ActionId action_id) = 0; + + // When set, `PageActionView::IsTriggerableEvent` uses this callback instead of + // the default logic. Pass nullopt to clear. + virtual void SetOverrideTriggerableEvent(actions::ActionId action_id, + std::optional event_flags) = 0;''' [[substitution]] @@ -38,4 +43,6 @@ replace = '''\1 void ClearOverrideChipColors(actions::ActionId action_id) override; void SetOverrideHeight(actions::ActionId action_id, int height) override; - void ClearOverrideHeight(actions::ActionId action_id) override;''' + void ClearOverrideHeight(actions::ActionId action_id) override; + void SetOverrideTriggerableEvent(actions::ActionId action_id, + std::optional event_flags) override;''' diff --git a/rewrite/chrome/browser/ui/views/page_action/page_action_model.h.toml b/rewrite/chrome/browser/ui/views/page_action/page_action_model.h.toml index dc83c7b2dd6..1db4fe0a2e0 100644 --- a/rewrite/chrome/browser/ui/views/page_action/page_action_model.h.toml +++ b/rewrite/chrome/browser/ui/views/page_action/page_action_model.h.toml @@ -15,11 +15,14 @@ replace = '''\1 bool always_show) = 0; virtual void SetOverrideHeight(base::PassKey, std::optional height) = 0; + virtual void SetOverrideTriggerableEvent(base::PassKey, + std::optional event_flags) = 0; virtual std::optional GetOverrideBackgroundColor() const = 0; virtual std::optional GetOverrideForegroundColor() const = 0; virtual bool GetAlwaysShowLabel() const = 0; - virtual std::optional GetOverrideHeight() const = 0;''' + virtual std::optional GetOverrideHeight() const = 0; + virtual std::optional GetOverrideTriggerableEvent() const = 0;''' [[substitution]] description = 'Implementing the interfaces in PageActionModel' @@ -34,16 +37,20 @@ replace = '''\1 bool always_show) override; void SetOverrideHeight(base::PassKey, std::optional height_px) override; + void SetOverrideTriggerableEvent(base::PassKey, + std::optional event_flags) override; std::optional GetOverrideBackgroundColor() const override; std::optional GetOverrideForegroundColor() const override; bool GetAlwaysShowLabel() const override; std::optional GetOverrideHeight() const override; + std::optional GetOverrideTriggerableEvent() const override; private: std::optional override_background_color_; std::optional override_foreground_color_; bool always_show_label_ = false; std::optional override_height_; + std::optional override_triggerable_event_flags_; public:''' @@ -53,4 +60,5 @@ re_pattern = '(\s+kShowRequested,)' replace = '''\1 kAlwaysShowLabel, kOverrideChipColors, - kOverrideHeight,''' + kOverrideHeight, + kOverrideTriggerableEvent,'''