From bfdba0e5ec440f3f31bf9c87aec12557a0861173 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 3 Jul 2018 16:12:55 -0700 Subject: [PATCH] Allow for a per-locale default search engine (value comes from config) Fixes https://github.com/brave/browser-laptop/issues/14647 --- app/locale.js | 13 ++--- app/sessionStore.js | 27 ++++++++- js/constants/appConfig.js | 2 +- js/constants/config.js | 8 +++ js/settings.js | 7 +++ test/unit/app/localeTest.js | 54 ++++++++++++++++++ test/unit/app/sessionStoreTest.js | 95 +++++++++++++++++++++++++++++++ 7 files changed, 194 insertions(+), 12 deletions(-) create mode 100644 test/unit/app/localeTest.js diff --git a/app/locale.js b/app/locale.js index 94b33690f42..1578c819c0d 100644 --- a/app/locale.js +++ b/app/locale.js @@ -385,24 +385,19 @@ availableLanguages.forEach(function (lang) { }) // Return the default locale in xx-XX format I.e. pt-BR -const defaultLocale = function () { - // If electron has the locale +module.exports.getDefaultLocale = function (allowUnsupported = false) { if (app.getLocale()) { - // Retrieve the language and convert _ to - var lang = app.getLocale().replace('_', '-') // If there is no country code designated use the language code if (!lang.match(/-/)) { lang = lang + '-' + lang.toUpperCase() } // If we have the language configured - if (configuredLanguages[lang]) { + if (allowUnsupported || configuredLanguages[lang]) { return lang - } else { - return DEFAULT_LANGUAGE } - } else { - return DEFAULT_LANGUAGE } + return DEFAULT_LANGUAGE } // Initialize translations for a language @@ -426,7 +421,7 @@ exports.init = function (language) { } // Currently selected language identifier I.e. 'en-US' - lang = language || defaultLocale() + lang = language || module.exports.getDefaultLocale() // Languages to support const langs = availableLanguages.map(function (lang) { diff --git a/app/sessionStore.js b/app/sessionStore.js index f5cfeaff2e4..83fd5695b4b 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -25,6 +25,7 @@ const {execSync} = require('child_process') const UpdateStatus = require('../js/constants/updateStatus') const settings = require('../js/constants/settings') const siteTags = require('../js/constants/siteTags') +const config = require('../js/constants/config') const downloadStates = require('../js/constants/downloadStates') const ledgerStatuses = require('./common/constants/ledgerStatuses') const promotionStatuses = require('./common/constants/promotionStatuses') @@ -1015,6 +1016,22 @@ module.exports.runImportDefaultSettings = (data) => { return data } +module.exports.setDefaultSearchEngine = (immutableData) => { + const defaultLocale = locale.getDefaultLocale(true) + let defaultSearchEngine = config.defaultSearchEngineByLocale.default + + for (let entry in config.defaultSearchEngineByLocale) { + if (entry === defaultLocale) { + defaultSearchEngine = config.defaultSearchEngineByLocale[entry] + break + } + } + + return defaultSearchEngine + ? immutableData.setIn(['settings', settings.DEFAULT_SEARCH_ENGINE], defaultSearchEngine) + : immutableData +} + /** * Loads the browser state from storage. * @@ -1079,9 +1096,15 @@ module.exports.loadAppState = () => { immutableData = module.exports.runPostMigrations(immutableData) } - locale.init(immutableData.getIn(['settings', settings.LANGUAGE])).then((locale) => { + locale.init(immutableData.getIn(['settings', settings.LANGUAGE])).then((lang) => { immutableData = setVersionInformation(immutableData) - app.setLocale(locale) + app.setLocale(lang) + + // Set default search engine for locale (if not already set) + if (immutableData.getIn(['settings', settings.DEFAULT_SEARCH_ENGINE]) == null) { + immutableData = module.exports.setDefaultSearchEngine(immutableData) + } + resolve(immutableData) }) }) diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index 24b72c41ec1..3bf67ec2a52 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -153,7 +153,7 @@ module.exports = { 'general.download-always-ask': true, 'general.spellcheck-enabled': true, 'general.spellcheck-languages': Immutable.fromJS(['en-US']), - 'search.default-search-engine': 'Google', + 'search.default-search-engine': null, 'search.offer-search-suggestions': false, // false by default for privacy reasons 'search.use-alternate-private-search-engine': false, 'search.use-alternate-private-search-engine-tor': true, diff --git a/js/constants/config.js b/js/constants/config.js index c720793d126..859b6a8f33d 100644 --- a/js/constants/config.js +++ b/js/constants/config.js @@ -42,6 +42,14 @@ module.exports = { defaultSearchSuggestions: false, maxHistorySites: 10 }, + // NOTE: names here correspond to `name` field in: + // js/data/searchProviders.js + defaultSearchEngineByLocale: { + // Example entries look like this: + // 'en-US': 'GitHub', + // 'es-MX': 'YouTube', + 'default': 'Google' + }, defaultOpenSearchPath: 'content/search/google.xml', vault: { syncUrl: (userId) => `${vaultHost}/v1/users/${userId}/appState`, diff --git a/js/settings.js b/js/settings.js index de6d97cd368..f0c468d5d71 100644 --- a/js/settings.js +++ b/js/settings.js @@ -5,6 +5,7 @@ const appConfig = require('./constants/appConfig') const Immutable = require('immutable') const settings = require('./constants/settings') +const config = require('./constants/config') const {passwordManagers, defaultPasswordManager, extensionIds, displayNames} = require('./constants/passwordManagers') const {bookmarksToolbarMode} = require('../app/common/constants/settingsEnums') @@ -54,8 +55,14 @@ const getDefaultSetting = (settingKey, settingsCollection) => { // 2) Get a default value when no value is set // allows for default to change until user locks it in + // + // These are overridden when: + // >> user picks their own setting in about:preferences#payments case settings.PAYMENTS_CONTRIBUTION_AMOUNT: return contributionDefaultAmount(settingKey, settingsCollection) + // >> locale is intialized (which determines default search engine) + case settings.DEFAULT_SEARCH_ENGINE: + return config.defaultSearchEngineByLocale.default } return undefined } diff --git a/test/unit/app/localeTest.js b/test/unit/app/localeTest.js new file mode 100644 index 00000000000..0e4f53d9f2a --- /dev/null +++ b/test/unit/app/localeTest.js @@ -0,0 +1,54 @@ +/* global describe, before, after, it */ +const mockery = require('mockery') +const assert = require('assert') + +require('../braveUnit') + +describe('locale unit tests', function () { + let locale + let testLocale = 'en-US' + const fakeElectron = require('../lib/fakeElectron') + + before(function () { + fakeElectron.app.getLocale = () => testLocale + + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + locale = require('../../../app/locale') + }) + + after(function () { + mockery.disable() + }) + + describe('getDefaultLocale', function () { + it('defaults to en-US if locale is falsey', function () { + testLocale = undefined + assert.equal(locale.getDefaultLocale(), 'en-US') + }) + + it('defaults to en-US if locale is not in supported locales/languages list', function () { + testLocale = 'not-a-real-locale' + assert.equal(locale.getDefaultLocale(), 'en-US') + }) + + it('ignores the supported locales/languages list if you pass `true`', function () { + testLocale = 'not-a-real-locale' + assert.equal(locale.getDefaultLocale(true), 'not-a-real-locale') + }) + + it('replaces underscore in locale with hyphen', function () { + testLocale = 'en_US' + assert.equal(locale.getDefaultLocale(), 'en-US') + }) + + it('puts a country code in place if one does not exist', function () { + testLocale = 'fr' + assert.equal(locale.getDefaultLocale(), 'fr-FR') + }) + }) +}) diff --git a/test/unit/app/sessionStoreTest.js b/test/unit/app/sessionStoreTest.js index 3837f1ad707..192f8dd921f 100644 --- a/test/unit/app/sessionStoreTest.js +++ b/test/unit/app/sessionStoreTest.js @@ -63,6 +63,14 @@ describe('sessionStore unit tests', function () { }, translation: (token) => { return token + }, + getDefaultLocale: (allowUnsupported = false) => { + return 'en-US' + } + } + const fakeConfig = { + defaultSearchEngineByLocale: { + default: 'MetaCrawler' } } @@ -77,6 +85,7 @@ describe('sessionStore unit tests', function () { mockery.registerMock('compare-versions', compareVersions) mockery.registerMock('electron', fakeElectron) mockery.registerMock('./locale', fakeLocale) + mockery.registerMock('../js/constants/config', fakeConfig) mockery.registerMock('./autofill', fakeAutofill) mockery.registerMock('./common/state/tabState', fakeTabState) mockery.registerMock('./common/state/windowState', fakeWindowState) @@ -912,6 +921,7 @@ describe('sessionStore unit tests', function () { let cleanAppDataStub let defaultAppStateSpy let runPostMigrationsSpy + let setDefaultSearchEngineSpy let localeInitSpy let backupSessionStub let runImportDefaultSettings @@ -922,6 +932,7 @@ describe('sessionStore unit tests', function () { cleanAppDataStub = sinon.stub(sessionStore, 'cleanAppData', (data) => data) defaultAppStateSpy = sinon.spy(sessionStore, 'defaultAppState') runPostMigrationsSpy = sinon.spy(sessionStore, 'runPostMigrations') + setDefaultSearchEngineSpy = sinon.spy(sessionStore, 'setDefaultSearchEngine') localeInitSpy = sinon.spy(fakeLocale, 'init') backupSessionStub = sinon.stub(sessionStore, 'backupSession') runImportDefaultSettings = sinon.spy(sessionStore, 'runImportDefaultSettings') @@ -933,6 +944,7 @@ describe('sessionStore unit tests', function () { runPreMigrationsSpy.restore() defaultAppStateSpy.restore() runPostMigrationsSpy.restore() + setDefaultSearchEngineSpy.restore() localeInitSpy.restore() backupSessionStub.restore() clearHSTSDataSpy.restore() @@ -1300,6 +1312,45 @@ describe('sessionStore unit tests', function () { assert.ok(false, 'promise was rejected: ' + JSON.stringify(result)) }) }) + + describe('when checking DEFAULT_SEARCH_ENGINE', function () { + beforeEach(function () { + setDefaultSearchEngineSpy.reset() + }) + + let readFileSyncStub + + afterEach(function () { + readFileSyncStub.restore() + }) + + it('calls setDefaultSearchEngine if DEFAULT_SEARCH_ENGINE is null', function () { + const session = { + settings: {} + } + readFileSyncStub = sinon.stub(fakeFileSystem, 'readFileSync').returns(JSON.stringify(session)) + return sessionStore.loadAppState() + .then(function (result) { + assert.equal(setDefaultSearchEngineSpy.calledOnce, true) + }, function (result) { + assert.ok(false, 'promise was rejected: ' + JSON.stringify(result)) + }) + }) + + it('does not call setDefaultSearchEngine if DEFAULT_SEARCH_ENGINE has a value', function () { + const session = { + settings: {} + } + session.settings[settings.DEFAULT_SEARCH_ENGINE] = 'Excite' + readFileSyncStub = sinon.stub(fakeFileSystem, 'readFileSync').returns(JSON.stringify(session)) + return sessionStore.loadAppState() + .then(function (result) { + assert.equal(setDefaultSearchEngineSpy.notCalled, true) + }, function (result) { + assert.ok(false, 'promise was rejected: ' + JSON.stringify(result)) + }) + }) + }) }) describe('backupSession', function () { @@ -1934,4 +1985,48 @@ describe('sessionStore unit tests', function () { }) }) }) + + describe('setDefaultSearchEngine', function () { + let getDefaultLocaleSpy + + beforeEach(function () { + getDefaultLocaleSpy = sinon.spy(fakeLocale, 'getDefaultLocale') + }) + + afterEach(function () { + getDefaultLocaleSpy.restore() + if (fakeConfig.defaultSearchEngineByLocale['en-US']) { + delete fakeConfig.defaultSearchEngineByLocale['en-US'] + } + if (!fakeConfig.defaultSearchEngineByLocale.default) { + fakeConfig.defaultSearchEngineByLocale.default = 'MetaCrawler' + } + }) + + it('calls getDefaultLocale with true', function () { + const input = Immutable.fromJS({settings: {}}) + sessionStore.setDefaultSearchEngine(input) + assert(getDefaultLocaleSpy.calledOnce) + }) + + it('defaults to `default` entry', function () { + const input = Immutable.fromJS({settings: {}}) + const output = sessionStore.setDefaultSearchEngine(input) + assert.equal(output.getIn(['settings', settings.DEFAULT_SEARCH_ENGINE]), 'MetaCrawler') + }) + + it('matches a locale specific entry (if present)', function () { + fakeConfig.defaultSearchEngineByLocale['en-US'] = 'Yahoo' + const input = Immutable.fromJS({settings: {}}) + const output = sessionStore.setDefaultSearchEngine(input) + assert.equal(output.getIn(['settings', settings.DEFAULT_SEARCH_ENGINE]), 'Yahoo') + }) + + it('does not change input if there is no default in config', function () { + delete fakeConfig.defaultSearchEngineByLocale.default + const input = Immutable.fromJS({settings: {}}) + const output = sessionStore.setDefaultSearchEngine(input) + assert.deepEqual(input, output) + }) + }) })