Skip to content

Commit

Permalink
Merge pull request #13670 from brave/adblock_removal
Browse files Browse the repository at this point in the history
Removed Adblock item from Browser app menu
  • Loading branch information
nullhook authored Jun 10, 2022
2 parents 703e7cd + e0a87bb commit 91a6dd2
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 35 deletions.
3 changes: 1 addition & 2 deletions app/brave_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
// Check chrome/app/chrome_command_ids.h when rebase.
// ID of IDC_BRAVE_COMANDS_START and first brave command should be same.
#define IDC_BRAVE_COMMANDS_START 56000
#define IDC_SHOW_BRAVE_REWARDS 56000
#define IDC_SHOW_BRAVE_ADBLOCK 56001
#define IDC_SHOW_BRAVE_REWARDS 56000
#define IDC_NEW_TOR_CONNECTION_FOR_SITE 56002
#define IDC_NEW_OFFTHERECORD_WINDOW_TOR 56003
#define IDC_CONTENT_CONTEXT_OPENLINKTOR 56004
Expand Down
3 changes: 0 additions & 3 deletions app/brave_generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,6 @@
<message name="IDS_SHOW_BRAVE_REWARDS" desc="The show Brave Rewards menu in the app menu">
Brave Rewards
</message>
<message name="IDS_SHOW_BRAVE_ADBLOCK" desc="The show Brave Ad Block menu in the app menu">
Brave Ad Block
</message>
<if expr="use_titlecase">
<message name="IDS_SHOW_BRAVE_WEBCOMPAT_REPORTER" desc="The menu item to report a broken site in the app menu">
Report a Broken Site
Expand Down
11 changes: 11 additions & 0 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,17 @@ bool BraveContentBrowserClient::HandleURLOverrideRewrite(
return true;
}

#if !BUILDFLAG(IS_ANDROID)
if (url->host() == kAdblockHost) {
GURL::Replacements replacements;
replacements.SetSchemeStr(content::kChromeUIScheme);
replacements.SetHostStr(chrome::kChromeUISettingsHost);
replacements.SetPathStr(kContentFiltersPath);
*url = url->ReplaceComponents(replacements);
return false;
}
#endif

// no special win10 welcome page
if (url->host() == chrome::kChromeUIWelcomeHost) {
*url = GURL(chrome::kChromeUIWelcomeURL);
Expand Down
27 changes: 26 additions & 1 deletion browser/brave_content_browser_client_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadChromeURL) {

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadCustomBravePages) {
std::vector<std::string> pages{
"adblock",
"ipfs-internals",
"rewards",
};

Expand Down Expand Up @@ -266,6 +266,31 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, RewriteChromeSync) {
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, RewriteAdblock) {
std::vector<std::string> schemes{
"brave://",
"chrome://",
};

for (const std::string& scheme : schemes) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(
ui_test_utils::NavigateToURL(browser(), GURL(scheme + "adblock")));
ASSERT_TRUE(WaitForLoadStop(contents));

EXPECT_STREQ(base::UTF16ToUTF8(
browser()->location_bar_model()->GetFormattedFullURL())
.c_str(),
"brave://settings/shields/content-filters");
EXPECT_EQ(browser()->location_bar_model()->GetURL(),
GURL("chrome://settings/shields/content-filters"));
EXPECT_EQ(
contents->GetController().GetLastCommittedEntry()->GetVirtualURL(),
GURL("chrome://settings/shields/content-filters"));
}
}

IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, RewriteMagnetURLURLBar) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
Expand Down
8 changes: 0 additions & 8 deletions browser/ui/brave_browser_command_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ void BraveBrowserCommandController::InitBraveCommandState() {
if (syncer::IsSyncAllowedByFlag())
UpdateCommandForBraveSync();
}
UpdateCommandForBraveAdblock();
UpdateCommandForWebcompatReporter();
#if BUILDFLAG(ENABLE_TOR)
UpdateCommandForTor();
Expand Down Expand Up @@ -165,10 +164,6 @@ void BraveBrowserCommandController::UpdateCommandForBraveRewards() {
UpdateCommandEnabled(IDC_SHOW_BRAVE_REWARDS, true);
}

void BraveBrowserCommandController::UpdateCommandForBraveAdblock() {
UpdateCommandEnabled(IDC_SHOW_BRAVE_ADBLOCK, true);
}

void BraveBrowserCommandController::UpdateCommandForWebcompatReporter() {
UpdateCommandEnabled(IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER, true);
}
Expand Down Expand Up @@ -259,9 +254,6 @@ bool BraveBrowserCommandController::ExecuteBraveCommandWithDisposition(
case IDC_SHOW_BRAVE_REWARDS:
brave::ShowBraveRewards(browser_);
break;
case IDC_SHOW_BRAVE_ADBLOCK:
brave::ShowBraveAdblock(browser_);
break;
case IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER:
brave::ShowWebcompatReporter(browser_);
break;
Expand Down
1 change: 0 additions & 1 deletion browser/ui/brave_browser_command_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class BraveBrowserCommandController : public chrome::BrowserCommandController

void InitBraveCommandState();
void UpdateCommandForBraveRewards();
void UpdateCommandForBraveAdblock();
void UpdateCommandForWebcompatReporter();
void UpdateCommandForBraveSync();
void UpdateCommandForBraveWallet();
Expand Down
6 changes: 0 additions & 6 deletions browser/ui/brave_browser_command_controller_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
// Test normal browser's brave commands status.
auto* command_controller = browser()->command_controller();
EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_REWARDS));
EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_ADBLOCK));

#if BUILDFLAG(ENABLE_TOR)
EXPECT_FALSE(
Expand Down Expand Up @@ -138,7 +137,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
auto* private_browser = CreateIncognitoBrowser();
auto* command_controller = private_browser->command_controller();
EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_REWARDS));
EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_ADBLOCK));

#if BUILDFLAG(ENABLE_TOR)
EXPECT_FALSE(
Expand Down Expand Up @@ -172,8 +170,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
auto* command_controller = guest_browser->command_controller();
EXPECT_FALSE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_REWARDS));

EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_ADBLOCK));

#if BUILDFLAG(ENABLE_TOR)
EXPECT_FALSE(
command_controller->IsCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE));
Expand Down Expand Up @@ -203,8 +199,6 @@ IN_PROC_BROWSER_TEST_F(BraveBrowserCommandControllerTest,
auto* command_controller = tor_browser->command_controller();
EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_REWARDS));

EXPECT_TRUE(command_controller->IsCommandEnabled(IDC_SHOW_BRAVE_ADBLOCK));

EXPECT_TRUE(
command_controller->IsCommandEnabled(IDC_NEW_TOR_CONNECTION_FOR_SITE));
EXPECT_TRUE(
Expand Down
10 changes: 2 additions & 8 deletions browser/ui/toolbar/brave_app_menu_model.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,6 @@ void BraveAppMenuModel::InsertBraveMenuItems() {
}
#endif

// Insert adblock menu at last. Assumed this is always enabled.
DCHECK(IsCommandIdEnabled(IDC_SHOW_BRAVE_ADBLOCK));
InsertItemWithStringIdAt(GetIndexOfBraveAdBlockItem(),
IDC_SHOW_BRAVE_ADBLOCK,
IDS_SHOW_BRAVE_ADBLOCK);

// Insert webcompat reporter item.
InsertItemWithStringIdAt(GetIndexOfCommandId(IDC_ABOUT),
IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER,
Expand Down Expand Up @@ -282,7 +276,7 @@ void BraveAppMenuModel::InsertBraveMenuItems() {
}
int index = IsCommandIdEnabled(IDC_SHOW_BRAVE_SYNC)
? GetIndexOfBraveSyncItem() + 1
: GetIndexOfBraveAdBlockItem();
: GetLastIndexOfSecondSection();
InsertSubMenuWithStringIdAt(index, IDC_APP_MENU_IPFS, IDS_APP_MENU_IPFS,
&ipfs_submenu_model_);

Expand Down Expand Up @@ -447,7 +441,7 @@ void BraveAppMenuModel::InsertAlternateProfileItems() {
InsertSeparatorAt(index, ui::NORMAL_SEPARATOR);
}

int BraveAppMenuModel::GetIndexOfBraveAdBlockItem() const {
int BraveAppMenuModel::GetLastIndexOfSecondSection() const {
// Insert as a last item in second section.
std::vector<int> commands_to_check = {IDC_SHOW_BRAVE_VPN_PANEL,
IDC_BRAVE_VPN_MENU,
Expand Down
2 changes: 1 addition & 1 deletion browser/ui/toolbar/brave_app_menu_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class BraveAppMenuModel : public AppMenuModel {
void InsertBraveMenuItems();
void InsertAlternateProfileItems();
int GetIndexOfBraveRewardsItem() const;
int GetIndexOfBraveAdBlockItem() const;
int GetLastIndexOfSecondSection() const;
int GetIndexOfBraveSyncItem() const;
int GetIndexOfBraveVPNItem() const;
#if BUILDFLAG(ENABLE_SIDEBAR)
Expand Down
5 changes: 1 addition & 4 deletions browser/ui/toolbar/brave_app_menu_model_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, MenuOrderTest) {
#if BUILDFLAG(ENABLE_BRAVE_VPN)
IDC_SHOW_BRAVE_VPN_PANEL,
#endif
IDC_SHOW_BRAVE_ADBLOCK,
IDC_ADD_NEW_PROFILE,
IDC_OPEN_GUEST_PROFILE,
IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER
Expand Down Expand Up @@ -193,7 +192,6 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, MenuOrderTest) {
IDC_SHOW_BRAVE_WALLET,
IDC_MANAGE_EXTENSIONS,
IDC_SHOW_BRAVE_SYNC,
IDC_SHOW_BRAVE_ADBLOCK,
IDC_ADD_NEW_PROFILE,
IDC_OPEN_GUEST_PROFILE,
IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER
Expand Down Expand Up @@ -226,7 +224,7 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, MenuOrderTest) {
DCHECK(guest_browser);
EXPECT_TRUE(guest_browser->profile()->IsGuestSession());
std::vector<int> commands_in_order_for_guest_profile = {
IDC_NEW_TAB, IDC_NEW_WINDOW, IDC_SHOW_DOWNLOADS, IDC_SHOW_BRAVE_ADBLOCK,
IDC_NEW_TAB, IDC_NEW_WINDOW, IDC_SHOW_DOWNLOADS,
IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER};
CheckCommandsAreInOrderInMenuModel(guest_browser,
commands_in_order_for_guest_profile);
Expand Down Expand Up @@ -267,7 +265,6 @@ IN_PROC_BROWSER_TEST_F(BraveAppMenuBrowserTest, MenuOrderTest) {
IDC_SHOW_DOWNLOADS,
IDC_SHOW_BRAVE_WALLET,
IDC_SHOW_BRAVE_SYNC,
IDC_SHOW_BRAVE_ADBLOCK,
IDC_ADD_NEW_PROFILE,
IDC_OPEN_GUEST_PROFILE,
IDC_SHOW_BRAVE_WEBCOMPAT_REPORTER};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const fingerprintModeOptions = [

function GlobalSettings () {
const onAdBlockListsClick = () => {
chrome.tabs.create({ url: 'chrome://adblock', active: true })
chrome.tabs.create({ url: 'chrome://settings/shields/content-filters', active: true })
}

const onSettingsClick = () => {
Expand Down
1 change: 1 addition & 0 deletions components/constants/webui_url_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ const char kShieldsPanelHost[] = "brave-shields.top-chrome";
const char kSidebarBookmarksHost[] = "sidebar-bookmarks.top-chrome";
const char kFederatedInternalsURL[] = "brave://federated-internals";
const char kFederatedInternalsHost[] = "federated-internals";
const char kContentFiltersPath[] = "shields/content-filters";
1 change: 1 addition & 0 deletions components/constants/webui_url_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@ extern const char kShieldsPanelHost[];
extern const char kSidebarBookmarksHost[];
extern const char kFederatedInternalsURL[];
extern const char kFederatedInternalsHost[];
extern const char kContentFiltersPath[];

#endif // BRAVE_COMPONENTS_CONSTANTS_WEBUI_URL_CONSTANTS_H_

0 comments on commit 91a6dd2

Please sign in to comment.