Merge pull request #7440 from brave/maxk-fix-brave-toolbar-buttons

cr88 followup: Fixes Bookmark & Speed Reader buttons.
brave · Jan 12, 2021 · 24c5ab8 · 24c5ab8
1 parent 7f3256e
commit 24c5ab8
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 19 deletions.
diff --git a/browser/ui/views/toolbar/bookmark_button.cc b/browser/ui/views/toolbar/bookmark_button.cc
@@ -20,8 +20,8 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/button/button.h"
 
-BookmarkButton::BookmarkButton()
-    : ToolbarButton(views::Button::PressedCallback()) {
+BookmarkButton::BookmarkButton(PressedCallback callback)
+    : ToolbarButton(std::move(callback)) {
   SetID(VIEW_ID_STAR_BUTTON);
   set_tag(IDC_BOOKMARK_THIS_TAB);
   SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FORWARD));

diff --git a/browser/ui/views/toolbar/bookmark_button.h b/browser/ui/views/toolbar/bookmark_button.h
@@ -6,24 +6,20 @@
 #ifndef BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BOOKMARK_BUTTON_H_
 #define BRAVE_BROWSER_UI_VIEWS_TOOLBAR_BOOKMARK_BUTTON_H_
 
-#include "base/scoped_observer.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
-#include "ui/base/models/simple_menu_model.h"
-#include "ui/views/widget/widget.h"
-#include "ui/views/widget/widget_observer.h"
 
 class BookmarkButton : public ToolbarButton {
  public:
-    BookmarkButton();
-    ~BookmarkButton() override;
+  BookmarkButton(PressedCallback callback);
+  ~BookmarkButton() override;
 
-    void SetToggled(bool on);
-    void UpdateImage();
+  void SetToggled(bool on);
+  void UpdateImage();
 
-    // ToolbarButton:
-    base::string16 GetTooltipText(const gfx::Point& p) const override;
-    const char* GetClassName() const override;
-    void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+  // ToolbarButton:
+  base::string16 GetTooltipText(const gfx::Point& p) const override;
+  const char* GetClassName() const override;
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
  private:
     bool active_ = false;

diff --git a/browser/ui/views/toolbar/brave_toolbar_view.cc b/browser/ui/views/toolbar/brave_toolbar_view.cc
@@ -11,18 +11,24 @@
 
 #include "base/bind.h"
 #include "base/feature_list.h"
+#include "brave/app/brave_command_ids.h"
 #include "brave/browser/ui/views/toolbar/bookmark_button.h"
 #include "brave/browser/ui/views/toolbar/speedreader_button.h"
 #include "brave/common/pref_names.h"
 #include "brave/components/speedreader/buildflags.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/layout_constants.h"
 #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/window_open_disposition.h"
+#include "ui/events/event.h"
 
 #if BUILDFLAG(ENABLE_SPEEDREADER)
 #include "brave/components/speedreader/features.h"
@@ -131,7 +137,14 @@ void BraveToolbarView::Init() {
       base::Bind(&BraveToolbarView::OnLocationBarIsWideChanged,
                  base::Unretained(this)));
 
-  bookmark_ = new BookmarkButton();
+  const auto callback = [](Browser* browser, int command,
+                           const ui::Event& event) {
+    chrome::ExecuteCommandWithDisposition(
+        browser, command, ui::DispositionFromEventFlags(event.flags()));
+  };
+
+  bookmark_ = new BookmarkButton(
+      base::BindRepeating(callback, browser_, IDC_BOOKMARK_THIS_TAB));
   bookmark_->SetTriggerableEventFlags(ui::EF_LEFT_MOUSE_BUTTON |
                                       ui::EF_MIDDLE_MOUSE_BUTTON);
   DCHECK(location_bar_);
@@ -141,7 +154,9 @@ void BraveToolbarView::Init() {
 #if BUILDFLAG(ENABLE_SPEEDREADER)
   // Speedreader.
   if (base::FeatureList::IsEnabled(speedreader::kSpeedreaderFeature)) {
-    speedreader_ = new SpeedreaderButton(profile->GetPrefs());
+    speedreader_ = new SpeedreaderButton(
+        base::BindRepeating(callback, browser_, IDC_TOGGLE_SPEEDREADER),
+        profile->GetPrefs());
     speedreader_->SetTriggerableEventFlags(ui::EF_LEFT_MOUSE_BUTTON |
                                            ui::EF_MIDDLE_MOUSE_BUTTON);
   }

diff --git a/browser/ui/views/toolbar/speedreader_button.cc b/browser/ui/views/toolbar/speedreader_button.cc
@@ -25,8 +25,9 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/button/button.h"
 
-SpeedreaderButton::SpeedreaderButton(PrefService* prefs)
-    : ToolbarButton(views::Button::PressedCallback()), prefs_(prefs) {
+SpeedreaderButton::SpeedreaderButton(PressedCallback callback,
+                                     PrefService* prefs)
+    : ToolbarButton(callback), prefs_(prefs) {
   SetID(BRAVE_VIEW_ID_SPEEDREADER_BUTTON);
   set_tag(IDC_TOGGLE_SPEEDREADER);
   SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FORWARD));

diff --git a/browser/ui/views/toolbar/speedreader_button.h b/browser/ui/views/toolbar/speedreader_button.h
@@ -19,7 +19,7 @@ class WebContents;
 // distilled.
 class SpeedreaderButton : public ToolbarButton {
  public:
-  explicit SpeedreaderButton(PrefService* prefs);
+  explicit SpeedreaderButton(PressedCallback callback, PrefService* prefs);
   ~SpeedreaderButton() override;
 
   SpeedreaderButton(const SpeedreaderButton&) = delete;