From 6fb489cd11b3d2ed083eedf3846da29a6083c541 Mon Sep 17 00:00:00 2001 From: Jocelyn Liu Date: Thu, 8 Aug 2019 10:28:03 -0700 Subject: [PATCH] Refactor patching based on review comments - Move adding Open Link with Tor into InitMenu and remove AppendBraveLinkItems - Fix bug when removing IDC_CONTENT_CONTEXT_TRANSLATE since it is not always existed - Define BRAVE_RENDER_VIEW_CONTEXT_MENU_H_ and BRAVE_TRANSLATE_BUBBLE_VIEW_H_ to be extensible in the header --- .../render_view_context_menu.cc | 38 ++++++++++--------- .../render_view_context_menu.h | 5 ++- .../views/translate/translate_bubble_view.h | 4 ++ ...ext_menu-render_view_context_menu.cc.patch | 12 ------ ...text_menu-render_view_context_menu.h.patch | 11 ++---- ...ws-translate-translate_bubble_view.h.patch | 16 ++++---- 6 files changed, 41 insertions(+), 45 deletions(-) delete mode 100644 patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc index 6f98b1319232..bb0439ee9352 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -37,21 +37,6 @@ BraveRenderViewContextMenu::BraveRenderViewContextMenu( : RenderViewContextMenu_Chromium(render_frame_host, params) { } -void RenderViewContextMenu_Chromium::AppendBraveLinkItems() { -} - -void BraveRenderViewContextMenu::AppendBraveLinkItems() { - if (!params_.link_url.is_empty()) { - const Browser* browser = GetBrowser(); - const bool is_app = browser && browser->is_app(); - - menu_model_.AddItemWithStringId( - IDC_CONTENT_CONTEXT_OPENLINKTOR, - is_app ? IDS_CONTENT_CONTEXT_OPENLINKTOR_INAPP - : IDS_CONTENT_CONTEXT_OPENLINKTOR); - } -} - bool BraveRenderViewContextMenu::IsCommandIdEnabled(int id) const { switch (id) { case IDC_CONTENT_CONTEXT_OPENLINKTOR: @@ -99,10 +84,29 @@ void BraveRenderViewContextMenu::AddSpellCheckServiceItem( void BraveRenderViewContextMenu::InitMenu() { RenderViewContextMenu_Chromium::InitMenu(); + + // Add Open Link with Tor + int index = -1; + if (!params_.link_url.is_empty()) { + const Browser* browser = GetBrowser(); + const bool is_app = browser && browser->is_app(); + + index = menu_model_.GetIndexOfCommandId( + IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD); + DCHECK_NE(index, -1); + + menu_model_.InsertItemWithStringIdAt( + index + 1, + IDC_CONTENT_CONTEXT_OPENLINKTOR, + is_app ? IDS_CONTENT_CONTEXT_OPENLINKTOR_INAPP + : IDS_CONTENT_CONTEXT_OPENLINKTOR); + } + // Only show the translate item when ENABLE_BRAVE_TRANSLATE (go-translate) is // enabled. #if !BUILDFLAG(ENABLE_BRAVE_TRANSLATE) - menu_model_.RemoveItemAt( - menu_model_.GetIndexOfCommandId(IDC_CONTENT_CONTEXT_TRANSLATE)); + index = menu_model_.GetIndexOfCommandId(IDC_CONTENT_CONTEXT_TRANSLATE); + if (index != -1) + menu_model_.RemoveItemAt(index); #endif } diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h index cb32946b7c4b..e9557df8e447 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h @@ -5,6 +5,10 @@ #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_RENDERER_CONTEXT_MENU_RENDER_VIEW_CONTEXT_MENU_H_ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_RENDERER_CONTEXT_MENU_RENDER_VIEW_CONTEXT_MENU_H_ +#define BRAVE_RENDER_VIEW_CONTEXT_MENU_H_ \ + private: \ + friend class BraveRenderViewContextMenu; \ + public: // Get the Chromium declaration. #define RenderViewContextMenu RenderViewContextMenu_Chromium #include "../../../../../chrome/browser/renderer_context_menu/render_view_context_menu.h" @@ -15,7 +19,6 @@ class BraveRenderViewContextMenu : public RenderViewContextMenu_Chromium { public: BraveRenderViewContextMenu(content::RenderFrameHost* render_frame_host, const content::ContextMenuParams& params); - void AppendBraveLinkItems() override; // RenderViewContextMenuBase: bool IsCommandIdEnabled(int command_id) const override; void ExecuteCommand(int id, int event_flags) override; diff --git a/chromium_src/chrome/browser/ui/views/translate/translate_bubble_view.h b/chromium_src/chrome/browser/ui/views/translate/translate_bubble_view.h index 7b74350b9c10..3e958d6490f0 100644 --- a/chromium_src/chrome/browser/ui/views/translate/translate_bubble_view.h +++ b/chromium_src/chrome/browser/ui/views/translate/translate_bubble_view.h @@ -6,6 +6,10 @@ #ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_TRANSLATE_TRANSLATE_BUBBLE_VIEW_H_ #define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_UI_VIEWS_TRANSLATE_TRANSLATE_BUBBLE_VIEW_H_ +#define BRAVE_TRANSLATE_BUBBLE_VIEW_H_ \ + private: \ + friend class BraveTranslateBubbleView; \ + public: class BraveTranslateBubbleView; #include "../../../../../../chrome/browser/ui/views/translate/translate_bubble_view.h" diff --git a/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch b/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch deleted file mode 100644 index 2abc88ae0b7f..000000000000 --- a/patches/chrome-browser-renderer_context_menu-render_view_context_menu.cc.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc -index 630436fb7b4ed95909bdc7c80da85b8ff524cd81..e4837ee01f811e74f0cca2353dea3826eff85f4a 100644 ---- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc -+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc -@@ -1163,6 +1163,7 @@ void RenderViewContextMenu::AppendLinkItems() { - : IDS_CONTENT_CONTEXT_OPENLINKOFFTHERECORD); - - AppendOpenInBookmarkAppLinkItems(); -+ AppendBraveLinkItems(); - AppendOpenWithLinkItems(); - - // While ChromeOS supports multiple profiles, only one can be open at a diff --git a/patches/chrome-browser-renderer_context_menu-render_view_context_menu.h.patch b/patches/chrome-browser-renderer_context_menu-render_view_context_menu.h.patch index c5d8fc03390e..df6e978bc652 100644 --- a/patches/chrome-browser-renderer_context_menu-render_view_context_menu.h.patch +++ b/patches/chrome-browser-renderer_context_menu-render_view_context_menu.h.patch @@ -1,15 +1,12 @@ diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.h b/chrome/browser/renderer_context_menu/render_view_context_menu.h -index 824741727052dfd87f8f65df8bbb5adf4a9f1e2e..04cef1566ff4067487211486ad0e2133e8eab585 100644 +index 824741727052dfd87f8f65df8bbb5adf4a9f1e2e..3caba335f84e5126c5912451b5dbcc807b21f62a 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu.h +++ b/chrome/browser/renderer_context_menu/render_view_context_menu.h -@@ -114,7 +114,10 @@ class RenderViewContextMenu : public RenderViewContextMenuBase { +@@ -113,6 +113,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase { + // Returns true if keyboard lock is active and requires the user to press and // hold escape to exit exclusive access mode. bool IsPressAndHoldEscRequiredToExitFullscreen() const; ++ BRAVE_RENDER_VIEW_CONTEXT_MENU_H_ -+ virtual void AppendBraveLinkItems(); -+ private: -+ friend class BraveRenderViewContextMenu; friend class RenderViewContextMenuTest; - friend class TestRenderViewContextMenu; - friend class FormatUrlForClipboardTest; diff --git a/patches/chrome-browser-ui-views-translate-translate_bubble_view.h.patch b/patches/chrome-browser-ui-views-translate-translate_bubble_view.h.patch index bf9d99ebf894..919f94219d05 100644 --- a/patches/chrome-browser-ui-views-translate-translate_bubble_view.h.patch +++ b/patches/chrome-browser-ui-views-translate-translate_bubble_view.h.patch @@ -1,12 +1,12 @@ diff --git a/chrome/browser/ui/views/translate/translate_bubble_view.h b/chrome/browser/ui/views/translate/translate_bubble_view.h -index 0eba3d7441cb352e81faea58548c87b40489f5a2..c5514bf7018a89c328bd11cfaaeecea7f9103d5b 100644 +index 0eba3d7441cb352e81faea58548c87b40489f5a2..9775f33cf6df16c01f837cf6d0defa41f20f3fc5 100644 --- a/chrome/browser/ui/views/translate/translate_bubble_view.h +++ b/chrome/browser/ui/views/translate/translate_bubble_view.h -@@ -143,6 +143,7 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView, - COMBOBOX_ID_TARGET_LANGUAGE, - }; +@@ -117,6 +117,7 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView, -+ friend class BraveTranslateBubbleView; - friend class TranslateBubbleViewTest; - friend void ::translate::test_utils::PressTranslate(::Browser*); - friend void ::translate::test_utils::PressRevert(::Browser*); + // Returns the current view state. + TranslateBubbleModel::ViewState GetViewState() const; ++ BRAVE_TRANSLATE_BUBBLE_VIEW_H_ + + protected: + // LocationBarBubbleDelegateView: