Skip to content

Commit

Permalink
Refactor patching based on review comments
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
yrliou committed Aug 8, 2019
1 parent 78dc52a commit 6fb489c
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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:

0 comments on commit 6fb489c

Please sign in to comment.