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 405bb42345c..4b028c29c33 100644 --- a/browser/ui/views/page_action/page_action_view_unittest.cc +++ b/browser/ui/views/page_action/page_action_view_unittest.cc @@ -266,4 +266,26 @@ TEST_F(PageActionViewTest, OverrideBackgroundColorReturnsModelValue) { EXPECT_EQ(page_action_view()->GetBackgroundColor(), SK_ColorRED); } +TEST_F(PageActionViewTest, OverrideBorderReturnsModelValue) { + EXPECT_CALL(*model(), GetOverrideBorder()) + .WillRepeatedly( + Return(std::optional(gfx::Insets::TLBR(1, 2, 3, 4)))); + + page_action_view()->OnPageActionModelChanged(*model()); + + EXPECT_EQ(page_action_view()->GetBorder()->GetInsets(), + gfx::Insets::TLBR(1, 2, 3, 4)); + + testing::Mock::VerifyAndClearExpectations(model()); + + // Even empty border should be respected + EXPECT_CALL(*model(), GetOverrideBorder()) + .WillRepeatedly(Return(std::optional(gfx::Insets()))); + + page_action_view()->OnPageActionModelChanged(*model()); + + EXPECT_EQ(page_action_view()->GetBorder()->GetInsets(), + gfx::Insets::TLBR(0, 0, 0, 0)); +} + } // 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 89f53f61d6d..ec543334cd7 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 @@ -178,6 +178,7 @@ void PartitionedStoragePageActionController::UpdatePageAction() { page_action_controller_->ClearOverrideHeight(kActionShowPartitionedStorage); page_action_controller_->SetOverrideTriggerableEvent( kActionShowPartitionedStorage, std::nullopt); + page_action_controller_->ClearOverrideBorder(kActionShowPartitionedStorage); return; } @@ -207,6 +208,10 @@ void PartitionedStoragePageActionController::UpdatePageAction() { page_action_controller_->SetOverrideHeight(kActionShowPartitionedStorage, 20); page_action_controller_->SetOverrideTriggerableEvent( kActionShowPartitionedStorage, ui::EF_RIGHT_MOUSE_BUTTON); + // Sets empty insets so that we can get rid of additional padding around icon + // and text. + page_action_controller_->SetOverrideBorder(kActionShowPartitionedStorage, + gfx::Insets()); } void PartitionedStoragePageActionController::OnPartitionedStorageMenuClosed() { diff --git a/chromium_src/chrome/browser/ui/page_actions/page_action_controller.cc b/chromium_src/chrome/browser/ui/page_actions/page_action_controller.cc index e132803b20e..b7271043b4f 100644 --- a/chromium_src/chrome/browser/ui/page_actions/page_action_controller.cc +++ b/chromium_src/chrome/browser/ui/page_actions/page_action_controller.cc @@ -52,6 +52,17 @@ void PageActionControllerImpl::SetOverrideTriggerableEvent( PageActionPassKey(), event_flags); } +void PageActionControllerImpl::SetOverrideBorder(actions::ActionId action_id, + const gfx::Insets& border) { + FindPageActionModel(action_id).SetOverrideBorder(PageActionPassKey(), border); +} + +void PageActionControllerImpl::ClearOverrideBorder( + actions::ActionId action_id) { + FindPageActionModel(action_id).SetOverrideBorder(PageActionPassKey(), + std::nullopt); +} + std::unique_ptr PageActionControllerImpl::CreateModel( actions::ActionId action_id, bool is_ephemeral) { diff --git a/chromium_src/chrome/browser/ui/page_actions/page_action_controller.h b/chromium_src/chrome/browser/ui/page_actions/page_action_controller.h index 9fcbb50240d..c21d010f51c 100644 --- a/chromium_src/chrome/browser/ui/page_actions/page_action_controller.h +++ b/chromium_src/chrome/browser/ui/page_actions/page_action_controller.h @@ -6,6 +6,10 @@ #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_CONTROLLER_H_ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_CONTROLLER_H_ +namespace gfx { +class Insets; +} // namespace gfx + namespace page_actions { class PageActionControllerImpl; } // namespace page_actions @@ -32,6 +36,9 @@ class PageActionControllerImpl void ClearOverrideHeight(actions::ActionId action_id); void SetOverrideTriggerableEvent(actions::ActionId action_id, std::optional event_flags); + void SetOverrideBorder(actions::ActionId action_id, + const gfx::Insets& border); + void ClearOverrideBorder(actions::ActionId action_id); // chromium_impl::PageActionControllerImpl: std::unique_ptr CreateModel( diff --git a/chromium_src/chrome/browser/ui/page_actions/page_action_model.cc b/chromium_src/chrome/browser/ui/page_actions/page_action_model.cc index 65b86510491..d670c7f961e 100644 --- a/chromium_src/chrome/browser/ui/page_actions/page_action_model.cc +++ b/chromium_src/chrome/browser/ui/page_actions/page_action_model.cc @@ -69,4 +69,17 @@ std::optional PageActionModel::GetOverrideTriggerableEvent() const { return override_triggerable_event_flags_; } +void PageActionModel::SetOverrideBorder(PageActionPassKey, + std::optional border) { + if (override_border_ == border) { + return; + } + override_border_ = border; + NotifyChange(Property::kOverrideBorder); +} + +std::optional PageActionModel::GetOverrideBorder() const { + return override_border_; +} + } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/page_actions/page_action_model.h b/chromium_src/chrome/browser/ui/page_actions/page_action_model.h index b01594fe4a6..e5d5966354b 100644 --- a/chromium_src/chrome/browser/ui/page_actions/page_action_model.h +++ b/chromium_src/chrome/browser/ui/page_actions/page_action_model.h @@ -6,6 +6,8 @@ #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_MODEL_H_ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_PAGE_ACTIONS_PAGE_ACTION_MODEL_H_ +#include "ui/gfx/geometry/insets.h" + namespace page_actions { class PageActionModel; } // namespace page_actions @@ -32,11 +34,14 @@ class PageActionModel : public chromium_impl::PageActionModel { std::optional height_px) override; void SetOverrideTriggerableEvent(PageActionPassKey, std::optional event_flags) override; + void SetOverrideBorder(PageActionPassKey, + std::optional border) 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; + std::optional GetOverrideBorder() const override; private: std::optional override_background_color_; @@ -44,6 +49,7 @@ class PageActionModel : public chromium_impl::PageActionModel { bool always_show_label_ = false; std::optional override_height_; std::optional override_triggerable_event_flags_; + std::optional override_border_; }; } // namespace page_actions diff --git a/chromium_src/chrome/browser/ui/page_actions/test_support/mock_page_action_model.h b/chromium_src/chrome/browser/ui/page_actions/test_support/mock_page_action_model.h index 44bd2a7b1f7..28bf8765782 100644 --- a/chromium_src/chrome/browser/ui/page_actions/test_support/mock_page_action_model.h +++ b/chromium_src/chrome/browser/ui/page_actions/test_support/mock_page_action_model.h @@ -46,6 +46,14 @@ class MockPageActionModel : public MockPageActionModel_Chromium { GetOverrideTriggerableEvent, (), (const, override)); + MOCK_METHOD(void, + SetOverrideBorder, + (PageActionPassKey, std::optional), + (override)); + MOCK_METHOD(std::optional, + GetOverrideBorder, + (), + (const, override)); }; } // 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 38c7a925799..e055c55cae6 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 @@ -15,6 +15,17 @@ #include "ui/views/style/platform_style.h" #include "ui/views/view_class_properties.h" +namespace { + +void MaybeOverrideBorder(const page_actions::PageActionModelInterface* source, + gfx::Insets& border_insets) { + if (source && source->GetOverrideBorder().has_value()) { + border_insets = source->GetOverrideBorder().value(); + } +} + +} // namespace + #define GetMinimumSize GetMinimumSize_Chromium #define OnNewActiveController OnNewActiveController_Chromium #define OnPageActionModelChanged OnPageActionModelChanged_Chromium @@ -71,6 +82,8 @@ void PageActionView::OnPageActionModelVisualRefresh( } else { ClearProperty(views::kCrossAxisAlignmentKey); } + + UpdateBorder(); } gfx::Size PageActionView::GetSizeForLabelWidth(int label_width) const { @@ -153,6 +166,7 @@ void PageActionView::OnPageActionModelChanged( SetTriggerableEventFlags(source->GetOverrideTriggerableEvent().value_or( ui::EF_LEFT_MOUSE_BUTTON)); } + UpdateBorder(); } } // namespace page_actions diff --git a/patches/chrome-browser-ui-page_actions-page_action_model.h.patch b/patches/chrome-browser-ui-page_actions-page_action_model.h.patch index 9910a165ee2..949d51f6005 100644 --- a/patches/chrome-browser-ui-page_actions-page_action_model.h.patch +++ b/patches/chrome-browser-ui-page_actions-page_action_model.h.patch @@ -1,8 +1,8 @@ diff --git a/chrome/browser/ui/page_actions/page_action_model.h b/chrome/browser/ui/page_actions/page_action_model.h -index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c83353aff35b 100644 +index 7dacb805a0c88fc8d78095f29550e70375639634..a8bb1e8007b1bf5ed6fc755084cc6e21bae7058e 100644 --- a/chrome/browser/ui/page_actions/page_action_model.h +++ b/chrome/browser/ui/page_actions/page_action_model.h -@@ -118,9 +118,25 @@ class PageActionModelInterface { +@@ -118,9 +118,28 @@ class PageActionModelInterface { virtual PageActionColorSource GetColorSource() const = 0; virtual bool IsEphemeral() const = 0; @@ -16,11 +16,14 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833 + std::optional height) = 0; + virtual void SetOverrideTriggerableEvent(PageActionPassKey, + std::optional event_flags) = 0; ++ virtual void SetOverrideBorder(PageActionPassKey, ++ std::optional border) = 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 std::optional GetOverrideBorder() const = 0; }; // PageActionModel represents the page action's state, scoped to a single tab. @@ -28,7 +31,7 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833 class PageActionModel : public PageActionModelInterface { public: explicit PageActionModel(actions::ActionId action_id, -@@ -217,6 +233,7 @@ class PageActionModel : public PageActionModelInterface { +@@ -217,6 +236,7 @@ class PageActionModel : public PageActionModelInterface { bool IsEphemeral() const override; private: @@ -36,7 +39,7 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833 // Identifies which property triggered a NotifyChange call, used for // per-property reentrancy checks. enum class Property { -@@ -239,7 +256,11 @@ class PageActionModel : public PageActionModelInterface { +@@ -239,7 +259,12 @@ class PageActionModel : public PageActionModelInterface { kAnchoredMessageActionIcon, kIsAnchoredMessageShowing, kAnchoredMessageIcon, @@ -45,11 +48,12 @@ index 7dacb805a0c88fc8d78095f29550e70375639634..3924212242cd280f61a77fe97bf2c833 + kOverrideChipColors, + kOverrideHeight, + kOverrideTriggerableEvent, -+ kMaxValue = kOverrideTriggerableEvent, ++ kOverrideBorder, ++ kMaxValue = kOverrideBorder, }; using PropertySet = base::EnumSet; -@@ -339,6 +360,7 @@ class PageActionModel : public PageActionModelInterface { +@@ -339,6 +364,7 @@ class PageActionModel : public PageActionModelInterface { base::ObserverList observer_list_; }; diff --git a/patches/chrome-browser-ui-views-page_action-page_action_view.cc.patch b/patches/chrome-browser-ui-views-page_action-page_action_view.cc.patch index 2d99a6bd850..7fe14c6f9c7 100644 --- a/patches/chrome-browser-ui-views-page_action-page_action_view.cc.patch +++ b/patches/chrome-browser-ui-views-page_action-page_action_view.cc.patch @@ -1,5 +1,5 @@ diff --git a/chrome/browser/ui/views/page_action/page_action_view.cc b/chrome/browser/ui/views/page_action/page_action_view.cc -index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..1a20372af0948f87a8d94a99ea287c039e10d126 100644 +index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..8051adc18865ad5e5d465de28336e51545b45ab8 100644 --- a/chrome/browser/ui/views/page_action/page_action_view.cc +++ b/chrome/browser/ui/views/page_action/page_action_view.cc @@ -167,7 +167,7 @@ void PageActionView::OnPageActionModelChanged( @@ -11,3 +11,11 @@ index 271f5b9adba10ee60aa45b4c00e1bfb541fd05fd..1a20372af0948f87a8d94a99ea287c03 if (model.GetShouldAnimateChipIn()) { AnimateIn(/*string_id=*/std::nullopt); } else { +@@ -232,6 +232,7 @@ void PageActionView::UpdateBorder() { + !observation_.GetSource()->GetImage().IsVectorIcon()) { + border_insets = GetInsetsForNonVectorIcon(); + } ++ MaybeOverrideBorder(observation_.GetSource(), border_insets); + SetBorder(views::CreateEmptyBorder(border_insets)); + } + diff --git a/rewrite/chrome/browser/ui/page_actions/page_action_model.h.toml b/rewrite/chrome/browser/ui/page_actions/page_action_model.h.toml index d427b8cd2bc..dbebe5701a8 100644 --- a/rewrite/chrome/browser/ui/page_actions/page_action_model.h.toml +++ b/rewrite/chrome/browser/ui/page_actions/page_action_model.h.toml @@ -26,11 +26,14 @@ replace = '''\1 std::optional height) = 0; virtual void SetOverrideTriggerableEvent(PageActionPassKey, std::optional event_flags) = 0; + virtual void SetOverrideBorder(PageActionPassKey, + std::optional border) = 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;\2''' + virtual std::optional GetOverrideTriggerableEvent() const = 0; + virtual std::optional GetOverrideBorder() const = 0;\2''' [[substitution]] description = ''' @@ -46,7 +49,8 @@ replace = '''\1 kAlwaysShowLabel, kOverrideChipColors, kOverrideHeight, kOverrideTriggerableEvent, - kMaxValue = kOverrideTriggerableEvent,''' + kOverrideBorder, + kMaxValue = kOverrideBorder,''' [[substitution]] description = '''Wrap PageActionModel in a chromium_impl namespace. diff --git a/rewrite/chrome/browser/ui/views/page_action/page_action_view.cc.toml b/rewrite/chrome/browser/ui/views/page_action/page_action_view.cc.toml index 73f1d35eba8..44e9be7ad84 100644 --- a/rewrite/chrome/browser/ui/views/page_action/page_action_view.cc.toml +++ b/rewrite/chrome/browser/ui/views/page_action/page_action_view.cc.toml @@ -7,3 +7,13 @@ description = 'Adding code to force show the label if it should be always shown in OnPageActionModelChanged()' re_pattern = 'else if \((model.ShouldShowSuggestionChip\(\))\)' replace = 'else if (\1 || model.GetAlwaysShowLabel())' + +[[substitution]] +description = ''' +Call MaybeOverrideBorder() in UpdateBorder() right before the SetBorder() call +in it. +''' +re_pattern = '(void PageActionView::UpdateBorder\(\).*?)(SetBorder\()' +re_flags = ['DOTALL'] +replace = '''\1MaybeOverrideBorder(observation_.GetSource(), border_insets); + \2'''