[brave-extension] add settings option to hide Shields activity count

fix brave/brave-browser#3121
brave · Dec 2, 2019 · db1ccb6 · db1ccb6
1 parent a04aeff
commit db1ccb6
Show file tree

Hide file tree

Showing 15 changed files with 84 additions and 26 deletions.
diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd
@@ -346,8 +346,14 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U
       <message name="IDS_SETTINGS_BRAVE_SHIELDS_NO_SCRIPT_CONTROL_LABEL" desc="Default Brave script blocking control setting label">
         Block scripts
       </message>
-      <message name="IDS_SETTINGS_BRAVE_SHIELDS_GOOGLE_LOGINS_LABEL" desc="Label for a switch control which allows Google logins">
-        Allow Google logins
+      <message name="IDS_SETTINGS_BRAVE_SHIELDS_LOOK_AND_FEEL_TITLE" desc="The title the settings section with interface options such as showing the resources blocked badge">
+        Look &amp; Feel
+      </message>
+      <message name="IDS_SETTINGS_BRAVE_SHIELDS_SHOW_STATS_BLOCKED_BADGE_LABEL" desc="Default Brave show stats blocked badge control setting label">
+        Show the number of blocked items on the Shields icon
+      </message>
+      <message name="IDS_SETTINGS_BRAVE_SHIELDS_GOOGLE_LOGIN_LABEL" desc="Label for a switch control which allows Google social buttons to be enabled/disabled">
+        Allow Google login buttons on third party sites
       </message>
       <message name="IDS_SETTINGS_BRAVE_SHIELDS_FACEBOOK_EMBEDDED_POSTS_LABEL" desc="Label for a switch control which allows Facebook embedded posts">
         Allow Facebook logins and embedded posts

diff --git a/browser/brave_profile_prefs.cc b/browser/brave_profile_prefs.cc
@@ -84,8 +84,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 
   registry->RegisterBooleanPref(kShieldsAdvancedViewEnabled,
                                 is_new_user == false);
-
-  // Google-oauth should work by default
+  registry->RegisterBooleanPref(kShieldsStatsBadgeVisible, true);
   registry->RegisterBooleanPref(kGoogleLoginControlType, true);
   registry->RegisterBooleanPref(kFBEmbedControlType, true);
   registry->RegisterBooleanPref(kTwitterEmbedControlType, true);

diff --git a/browser/brave_profile_prefs_browsertest.cc b/browser/brave_profile_prefs_browsertest.cc
@@ -33,6 +33,9 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest, MiscBravePrefs) {
       kHTTPSEVerywhereControlType));
   EXPECT_FALSE(
       browser()->profile()->GetPrefs()->GetBoolean(kNoScriptControlType));
+  EXPECT_TRUE(
+      browser()->profile()->GetPrefs()->GetBoolean(
+        kShieldsStatsBadgeVisible));
   EXPECT_TRUE(
       browser()->profile()->GetPrefs()->GetBoolean(kAdControlType));
   EXPECT_TRUE(

diff --git a/browser/extensions/api/settings_private/brave_prefs_util.cc b/browser/extensions/api/settings_private/brave_prefs_util.cc
@@ -42,6 +42,8 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetWhitelistedKeys() {
   // Default Brave shields
   (*s_brave_whitelist)[kShieldsAdvancedViewEnabled] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  (*s_brave_whitelist)[kShieldsStatsBadgeVisible] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_brave_whitelist)[kAdControlType] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_brave_whitelist)[kHTTPSEVerywhereControlType] =

diff --git a/browser/resources/settings/default_brave_shields_page/default_brave_shields_page.html b/browser/resources/settings/default_brave_shields_page/default_brave_shields_page.html
@@ -38,6 +38,9 @@
          margin: 0 20px;
          cursor: pointer;
        }
+       .first-after-text {
+         border-top: 0;
+       }
     </style>
     <div class="settings-box first">
       <div class="flex">
@@ -129,6 +132,13 @@
         </template>
       </select>
     </div>
+    <div class="settings-box">
+      <div class="label shields-primary-title">$i18n{lookFeelTitle}</div>
+    </div>
+    <settings-toggle-button
+        class="first-after-text"
+        pref="{{prefs.brave.shields.stats_badge_visible}}"
+        label="$i18n{showStatsBlockedBadgeLabel}">
   </template>
   <script src="default_brave_shields_page.js"></script>
 </dom-module>
diff --git a/chromium_src/chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc b/chromium_src/chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc
@@ -99,9 +99,13 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
       IDS_SETTINGS_BRAVE_SHIELDS_HTTPS_EVERYWHERE_CONTROL_LABEL},
     {"noScriptControlLabel",
       IDS_SETTINGS_BRAVE_SHIELDS_NO_SCRIPT_CONTROL_LABEL},
-   {"googleLoginControlLabel",
-      IDS_SETTINGS_BRAVE_SHIELDS_GOOGLE_LOGINS_LABEL},
-   {"fbEmbedControlLabel",
+    {"lookFeelTitle",
+      IDS_SETTINGS_BRAVE_SHIELDS_LOOK_AND_FEEL_TITLE},
+    {"showStatsBlockedBadgeLabel",
+      IDS_SETTINGS_BRAVE_SHIELDS_SHOW_STATS_BLOCKED_BADGE_LABEL},
+    {"googleLoginControlLabel",
+      IDS_SETTINGS_BRAVE_SHIELDS_GOOGLE_LOGIN_LABEL},
+    {"fbEmbedControlLabel",
       IDS_SETTINGS_BRAVE_SHIELDS_FACEBOOK_EMBEDDED_POSTS_LABEL},
     {"twitterEmbedControlLabel",
       IDS_SETTINGS_BRAVE_SHIELDS_TWITTER_EMBEDDED_TWEETS_LABEL},

diff --git a/common/pref_names.cc b/common/pref_names.cc
@@ -47,6 +47,8 @@ const char kHTTPSEVerywhereControlType[] = "brave.https_everywhere_default";
 const char kNoScriptControlType[] = "brave.no_script_default";
 const char kShieldsAdvancedViewEnabled[] =
     "brave.shields.advanced_view_enabled";
+const char kShieldsStatsBadgeVisible[] =
+    "brave.shields.stats_badge_visible";
 const char kAdControlType[] = "brave.ad_default";
 const char kGoogleLoginControlType[] = "brave.google_login_default";
 const char kFBEmbedControlType[] = "brave.fb_embed_default";

diff --git a/common/pref_names.h b/common/pref_names.h
@@ -40,6 +40,7 @@ extern const char kReferralAndroidFirstRunTimestamp[];
 extern const char kHTTPSEVerywhereControlType[];
 extern const char kNoScriptControlType[];
 extern const char kShieldsAdvancedViewEnabled[];
+extern const char kShieldsStatsBadgeVisible[];
 extern const char kAdControlType[];
 extern const char kGoogleLoginControlType[];
 extern const char kFBEmbedControlType[];

diff --git a/components/brave_extension/extension/brave_extension/background/api/browserActionAPI.ts b/components/brave_extension/extension/brave_extension/background/api/browserActionAPI.ts
@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { isHttpOrHttps } from '../../helpers/urlUtils'
+import { getViewPreferences } from './shieldsAPI'
 
 // TODO: use `import` for these assets once the webpack build
 // for brave extension allows dynamic file serving like other brave-core
@@ -48,12 +49,16 @@ export function init () {
  * @param {string} text - The text to put on the badge
  */
 export const setBadgeText = (tabId: number, text: string) => {
-  if (chrome.browserAction) {
-    chrome.browserAction.setBadgeText({
-      tabId,
-      text: String(text)
+  getViewPreferences()
+    .then((settings: chrome.braveShields.BraveShieldsViewPreferences) => {
+      if (chrome.browserAction) {
+        chrome.browserAction.setBadgeText({
+          tabId,
+          text: settings.statsBadgeVisible ? String(text) : ''
+        })
+      }
     })
-  }
+    .catch((error: any) => console.error('[Shields]: can\'t get preferences data', error))
 }
 
 /**

diff --git a/components/brave_extension/extension/brave_extension/background/api/shieldsAPI.ts b/components/brave_extension/extension/brave_extension/background/api/shieldsAPI.ts
@@ -172,23 +172,32 @@ export const reportBrokenSite = (tabId: number) =>
 
 export type GetViewPreferencesData = {
   showAdvancedView: boolean
+  statsBadgeVisible: boolean
 }
 
 const settingsKeys = {
-  showAdvancedView: { key: 'brave.shields.advanced_view_enabled', type: chrome.settingsPrivate.PrefType.BOOLEAN }
+  showAdvancedView: { key: 'brave.shields.advanced_view_enabled', type: chrome.settingsPrivate.PrefType.BOOLEAN },
+  statsBadgeVisible: { key: 'brave.shields.stats_badge_visible', type: chrome.settingsPrivate.PrefType.BOOLEAN },
 }
 export async function getViewPreferences (): Promise<GetViewPreferencesData> {
   const showAdvancedViewPref = await SettingsPrivate.getPreference(settingsKeys.showAdvancedView.key)
+  const statsBadgeVisiblePref = await SettingsPrivate.getPreference(settingsKeys.statsBadgeVisible.key)
+
   if (showAdvancedViewPref.type !== settingsKeys.showAdvancedView.type) {
     throw new Error(`Unexpected settings type received for "${settingsKeys.showAdvancedView.key}". Expected: ${settingsKeys.showAdvancedView.type}, Received: ${showAdvancedViewPref.type}`)
   }
+  if (statsBadgeVisiblePref.type !== settingsKeys.statsBadgeVisible.type) {
+    throw new Error(`Unexpected settings type received for "${settingsKeys.statsBadgeVisible.key}". Expected: ${settingsKeys.statsBadgeVisible.type}, Received: ${statsBadgeVisiblePref.type}`)
+  }
   return {
-    showAdvancedView: showAdvancedViewPref.value
+    showAdvancedView: showAdvancedViewPref.value,
+    statsBadgeVisible: statsBadgeVisiblePref.value
   }
 }
 
 export type SetViewPreferencesData = {
   showAdvancedView?: boolean
+  statsBadgeVisible?: boolean
 }
 export async function setViewPreferences (preferences: SetViewPreferencesData): Promise<void> {
   const setOps = []
@@ -197,6 +206,11 @@ export async function setViewPreferences (preferences: SetViewPreferencesData):
       SettingsPrivate.setPreference(settingsKeys.showAdvancedView.key, preferences.showAdvancedView)
     )
   }
+  if (preferences.statsBadgeVisible !== undefined) {
+    setOps.push(
+      SettingsPrivate.setPreference(settingsKeys.statsBadgeVisible.key, preferences.statsBadgeVisible)
+    )
+  }
   await Promise.all(setOps)
 }
 

diff --git a/components/brave_extension/extension/brave_extension/braveShieldsPanel.tsx b/components/brave_extension/extension/brave_extension/braveShieldsPanel.tsx
@@ -45,7 +45,7 @@ Promise.all([
     mountNode
   )
 })
-.catch((e) => {
+.catch((error: any) => {
   console.error('Problem mounting brave shields')
-  console.error(e)
+  console.error(error)
 })
diff --git a/components/definitions/chromel.d.ts b/components/definitions/chromel.d.ts
@@ -214,9 +214,11 @@ declare namespace chrome.braveShields {
 
   type BraveShieldsViewPreferences = {
     showAdvancedView: boolean
+    statsBadgeVisible: boolean
   }
   type BraveShieldsSetViewPreferencesData = {
     showAdvancedView?: boolean
+    statsBadgeVisible?: boolean
   }
 }
 

diff --git a/components/test/brave_extension/background/api/browserActionAPI_test.ts b/components/test/brave_extension/background/api/browserActionAPI_test.ts
@@ -3,24 +3,28 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import * as browserActionAPI from '../../../../brave_extension/extension/brave_extension/background/api/browserActionAPI'
+import * as shieldsAPI from '../../../../brave_extension/extension/brave_extension/background/api/shieldsAPI'
 
 describe('BrowserAction API', () => {
   describe('setBadgeText', () => {
-    let spy: jest.SpyInstance
+    let getPreferencesSpy: jest.SpyInstance
+    let setBadgeTextSpy: jest.SpyInstance
     const text = '42'
     const tabId = 1337
     beforeEach(() => {
-      spy = jest.spyOn(chrome.browserAction, 'setBadgeText')
+      getPreferencesSpy = jest.spyOn(shieldsAPI, 'getViewPreferences')
+      setBadgeTextSpy = jest.spyOn(chrome.browserAction, 'setBadgeText')
       browserActionAPI.setBadgeText(tabId, text)
     })
     afterEach(() => {
-      spy.mockRestore()
+      getPreferencesSpy.mockRestore()
+      setBadgeTextSpy.mockRestore()
     })
-    it('calls chrome.browserAction.setBadgeText with the text', () => {
-      expect(spy).toHaveBeenCalled()
-      expect(spy.mock.calls[0][0]).toEqual({
-        tabId,
-        text
+    it('calls chrome.browserAction.setBadgeText with the text', (cb) => {
+      getPreferencesSpy().then(() => {
+        expect.assertions(1)
+        expect(setBadgeTextSpy).toBeCalledWith({ tabId, text })
+        cb()
       })
     })
   })

diff --git a/components/test/brave_extension/state/shieldsPanelState_test.ts b/components/test/brave_extension/state/shieldsPanelState_test.ts
@@ -7,6 +7,7 @@ import * as deepFreeze from 'deep-freeze-node'
 import * as shieldsPanelState from '../../../brave_extension/extension/brave_extension/state/shieldsPanelState'
 import * as noScriptState from '../../../brave_extension/extension/brave_extension/state/noScriptState'
 import * as shieldsAPI from '../../../brave_extension/extension/brave_extension/background/api/shieldsAPI'
+import * as browserActionAPI from '../../../brave_extension/extension/brave_extension/background/api/browserActionAPI'
 
 const state: State = deepFreeze({
   currentWindowId: 1,
@@ -383,15 +384,15 @@ describe('shieldsPanelState test', () => {
   describe('updateShieldsIconBadgeText', () => {
     let spy: jest.SpyInstance
     beforeEach(() => {
-      spy = jest.spyOn(chrome.browserAction, 'setBadgeText')
+      spy = jest.spyOn(browserActionAPI, 'setBadgeText')
     })
     afterEach(() => {
       spy.mockRestore()
     })
     it('calls setBadgeText if tab exists', () => {
       shieldsPanelState.updateShieldsIconBadgeText(state)
       expect(spy).toHaveBeenCalled()
-      expect(spy.mock.calls[0][0]).toEqual({ tabId: 2, text: '' })
+      expect(spy.mock.calls[0][0]).toEqual(2)
     })
     it('does not call setBadgeText if tab does not exist', () => {
       const newState: State = deepFreeze({

diff --git a/components/test/testData.ts b/components/test/testData.ts
@@ -122,6 +122,11 @@ let mockSettings: MockSettingsStore = {
     key: 'brave.shields.advanced_view_enabled',
     type: 'BOOLEAN',
     value: false
+  },
+  ['brave.shields.stats_badge_visible']: {
+    key: 'brave.shields.stats_badge_visible',
+    type: 'BOOLEAN',
+    value: true
   }
 }