Finishing up settings for newtab:

Fixes #2106

- pulled out setting values into an enum
- position moved to below "Brave starts with" (in preferences.js)
- resolved circular dependency issue in appUrlUtil which broke app
- made a newtab option called BLANK and renamed value TEMP. We can change back when ready to go live
- defaultUrl removed from main.js (since it gets defaulted in windowStore.js)
- removed defaultUrl from window.js (since windowStore.js also handles this)
- put about pages into config; updated contextMenus.js to use config constants
- adding missing "base" element to searchProviders that had it missing (mdn, github, startpage, etc)
- updated defaultUrl logic in appUrlUtil.js to default unknown values to about:blank
- windowStore.js now uses new defaultUrl method (from appUrlUtil) instead of config.defaultUrl

Auditors: @kingscott @cezaraugusto

app/common/constants/settingsEnums.js

@@ -0,0 +1,28 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const startsWithOption = {
+ WINDOWS_TABS_FROM_LAST_TIME: 'lastTime',
+ HOMEPAGE: 'homePage',
+ NEW_TAB_PAGE: 'newTabPage'
+}
+
+const newTabMode = {
+ BLANK: 'blank',
+ NEW_TAB_PAGE: 'newTabPage',
+ HOMEPAGE: 'homePage',
+ DEFAULT_SEARCH_ENGINE: 'defaultSearchEngine'
+}
+
+const bookmarksToolbarMode = {
+ TEXT_ONLY: 'textOnly',
+ TEXT_AND_FAVICONS: 'textAndFavicons',
+ FAVICONS_ONLY: 'faviconsOnly'
+}
+
+module.exports = {
+ startsWithOption,
+ newTabMode,
+ bookmarksToolbarMode
+}

app/extensions/brave/locales/en-US/preferences.properties

@@ -103,9 +103,12 @@ startsWith=Brave starts with
 startsWithOptionLastTime=my windows / tabs from last time
 startsWithOptionHomePage=my home page
 startsWithOptionNewTabPage=the new tab page
-startsWithOptionDefaultSearchEngine=default search engine
+newTabMode=A new tab shows
+newTabBlank=blank
+newTabNewTabPage=the new tab page
+newTabHomePage=my home page
+newTabDefaultSearchEngine=default search engine
 myHomepage=My homepage is
-newTabMode=A new tab is
 default=Default
 searchEngine=Search Engine
 engineGoKey=Engine Go Key (Type First)

docs/state.md

@@ -161,7 +161,7 @@ AppStore
  // See defaults in js/constants/appConfig.js
  'general.startup-mode': string, // One of: lastTime, homePage, newTabPage
  'general.homepage': string, // URL of the user's homepage
- 'general.newtab-mode': string, // One of: newTabPage, homePage, defaultSearchEngine
+ 'general.newtab-mode-TEMP': string, // One of: blank, newTabPage, homePage, defaultSearchEngine
  'general.show-home-button': boolean, // true if the home button should be shown
  'general.useragent.value': (undefined|string), // custom user agent value
  'general.downloads.default-save-path': string, // default path for saving files

js/about/preferences.js

@@ -20,7 +20,8 @@ const messages = require('../constants/messages')
 const settings = require('../constants/settings')
 const coinbaseCountries = require('../constants/coinbaseCountries')
 const {passwordManagers, extensionIds} = require('../constants/passwordManagers')
-const bookmarksToolbarMode = require('../../app/common/constants/bookmarksToolbarMode')
+const {startsWithOption, newTabMode, bookmarksToolbarMode} = require('../../app/common/constants/settingsEnums')
+
 const WidevineInfo = require('../../app/renderer/components/widevineInfo')
 const aboutActions = require('./aboutActions')
 const getSetting = require('../settings').getSetting
@@ -646,9 +647,18 @@ class GeneralTab extends ImmutableComponent {
  <SettingItem dataL10nId='startsWith'>
  <select value={getSetting(settings.STARTUP_MODE, this.props.settings)}
  onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.STARTUP_MODE)} >
- <option data-l10n-id='startsWithOptionLastTime' value='lastTime' />
- <option data-l10n-id='startsWithOptionHomePage' value='homePage' />
- <option data-l10n-id='startsWithOptionNewTabPage' value='newTabPage' />
+ <option data-l10n-id='startsWithOptionLastTime' value={startsWithOption.WINDOWS_TABS_FROM_LAST_TIME} />
+ <option data-l10n-id='startsWithOptionHomePage' value={startsWithOption.HOMEPAGE} />
+ <option data-l10n-id='startsWithOptionNewTabPage' value={startsWithOption.NEW_TAB_PAGE} />
+ </select>
+ </SettingItem>
+ <SettingItem dataL10nId='newTabMode'>
+ <select value={getSetting(settings.NEWTAB_MODE, this.props.settings)}
+ onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.NEWTAB_MODE)} >
+ <option data-l10n-id='newTabBlank' value={newTabMode.BLANK} />
+ <option data-l10n-id='newTabNewTabPage' value={newTabMode.NEW_TAB_PAGE} />
+ <option data-l10n-id='newTabHomePage' value={newTabMode.HOMEPAGE} />
+ <option data-l10n-id='newTabDefaultSearchEngine' value={newTabMode.DEFAULT_SEARCH_ENGINE} />
  </select>
  </SettingItem>
  <SettingItem dataL10nId='myHomepage'>
@@ -675,14 +685,6 @@ class GeneralTab extends ImmutableComponent {
  prefKey={settings.SHOW_BOOKMARKS_TOOLBAR} settings={this.props.settings}
  onChangeSetting={this.props.onChangeSetting} />
  </SettingItem>
- <SettingItem dataL10nId='newTabMode'>
- <select value={getSetting(settings.NEWTAB_MODE, this.props.settings)}
- onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.NEWTAB_MODE)} >
- <option data-l10n-id='startsWithOptionNewTabPage' value='newTabPage' />
- <option data-l10n-id='startsWithOptionHomePage' value='homePage' />
- <option data-l10n-id='startsWithOptionDefaultSearchEngine' value='defaultSearchEngine' />
- </select>
- </SettingItem>
  <SettingItem dataL10nId='selectedLanguage'>
  <select value={getSetting(settings.LANGUAGE, this.props.settings) || defaultLanguage}
  onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.LANGUAGE)} >

js/components/main.js

@@ -47,7 +47,6 @@ const WindowCaptionButtons = require('../../app/renderer/components/windowCaptio
 const CheckDefaultBrowserDialog = require('../../app/renderer/components/checkDefaultBrowserDialog')
 
 // Constants
-const config = require('../constants/config')
 const appConfig = require('../constants/appConfig')
 const messages = require('../constants/messages')
 const settings = require('../constants/settings')
@@ -56,7 +55,7 @@ const dragTypes = require('../constants/dragTypes')
 const keyCodes = require('../../app/common/constants/keyCodes')
 const keyLocations = require('../../app/common/constants/keyLocations')
 const isWindows = process.platform === 'win32'
-const bookmarksToolbarMode = require('../../app/common/constants/bookmarksToolbarMode')
+const {bookmarksToolbarMode} = require('../../app/common/constants/settingsEnums')
 
 // State handling
 const basicAuthState = require('../../app/common/state/basicAuthState')
@@ -321,9 +320,8 @@ class Main extends ImmutableComponent {
  }
  }
  let openInForeground = getSetting(settings.SWITCH_TO_NEW_TABS) === true || options.openInForeground
- // let defaultUrl = newFrameUrl()
  const frameOpts = {
- location: url || config.defaultUrl,
+ location: url,
  isPrivate: !!options.isPrivate,
  isPartitioned: !!options.isPartitioned,
  parentFrameKey: options.parentFrameKey

js/components/tabs.js

@@ -12,10 +12,6 @@ const windowStore = require('../stores/windowStore')
 const dragTypes = require('../constants/dragTypes')
 const cx = require('../lib/classSet')
 
-const settings = require('../constants/settings')
-const getSetting = require('../settings').getSetting
-const searchProviders = require('../data/searchProviders').providers
-
 const Button = require('./button')
 const Tab = require('./tab')
 const dnd = require('../dnd')

js/components/window.js

@@ -11,7 +11,6 @@ const appStoreRenderer = require('../stores/appStoreRenderer')
 const windowActions = require('../actions/windowActions')
 const Main = require('./main')
 const SiteTags = require('../constants/siteTags')
-const config = require('../constants/config')
 const cx = require('../lib/classSet')
 const {getPlatformStyles} = require('../../app/common/lib/platformUtil')
 
@@ -41,9 +40,7 @@ class Window extends React.Component {
  componentWillMount () {
  if (!this.props.initWindowState || this.props.initWindowState.frames.length === 0) {
  if (this.props.frames.length === 0) {
- windowActions.newFrame({
- location: config.defaultUrl
- })
+ windowActions.newFrame()
  } else {
  this.props.frames.forEach((frame) => {
  windowActions.newFrame(frame)

js/constants/appConfig.js

@@ -99,7 +99,7 @@ module.exports = {
  'general.language': null, // null means to use the OS lang
  'general.startup-mode': 'lastTime',
  'general.homepage': 'https://www.brave.com',
- 'general.newtab-mode': 'newTabPage',
+ 'general.newtab-mode-TEMP': 'blank',
  'general.show-home-button': false,
  'general.useragent.value': null, // Set at runtime
  'general.autohide-menu': true,

js/constants/config.js

@@ -23,6 +23,12 @@ module.exports = {
  },
  defaultLocale: 'en-US',
  defaultUrl: 'about:newtab',
+ aboutPages: {
+ blank: 'about:blank',
+ newtab: 'about:newtab',
+ bookmarks: 'about:bookmarks',
+ history: 'about:history'
+ },
  urlBarSuggestions: {
  maxOpenedFrames: 2,
  maxBookmarkSites: 2,

js/constants/settings.js

@@ -6,7 +6,7 @@ const settings = {
  // General tab
  STARTUP_MODE: 'general.startup-mode',
  HOMEPAGE: 'general.homepage',
- NEWTAB_MODE: 'general.newtab-mode',
+ NEWTAB_MODE: 'general.newtab-mode-TEMP',
  SHOW_HOME_BUTTON: 'general.show-home-button',
  USERAGENT: 'general.useragent.value',
  DEFAULT_DOWNLOAD_SAVE_PATH: 'general.downloads.default-save-path',

js/contextMenus.js

@@ -34,7 +34,7 @@ const urlParse = require('url').parse
 const eventUtil = require('./lib/eventUtil')
 const currentWindow = require('../app/renderer/currentWindow')
 const config = require('./constants/config')
-const bookmarksToolbarMode = require('../app/common/constants/bookmarksToolbarMode')
+const {bookmarksToolbarMode} = require('../app/common/constants/settingsEnums')
 const extensionState = require('../app/common/state/extensionState')
 const extensionActions = require('../app/common/actions/extensionActions')
 const appStore = require('./stores/appStoreRenderer')
@@ -403,7 +403,7 @@ function moreBookmarksTemplateInit (allBookmarkItems, bookmarks, activeFrame) {
  template.push({
  l10nLabelId: 'moreBookmarks',
  click: function () {
- windowActions.newFrame({ location: 'about:bookmarks' })
+ windowActions.newFrame({ location: config.aboutPages.bookmarks })
  windowActions.setContextMenuDetail()
  }
  })
@@ -463,7 +463,7 @@ function tabTemplateInit (frameProps) {
  const frameKey = frameProps.get('key')
  const items = [CommonMenu.newTabMenuItem(frameProps.get('key'))]
  const location = frameProps.get('location')
- if (location !== 'about:newtab') {
+ if (location !== config.aboutPages.newtab) {
  items.push(
  CommonMenu.separatorMenuItem,
  {
@@ -487,7 +487,7 @@ function tabTemplateInit (frameProps) {
 
  if (!frameProps.get('isPrivate')) {
  const isPinned = frameProps.get('pinnedLocation')
- if (!(location === 'about:blank' || location === 'about:newtab' || isIntermediateAboutPage(location))) {
+ if (!(location === config.aboutPages.blank || location === config.aboutPages.newtab || isIntermediateAboutPage(location))) {
  items.push({
  label: locale.translation(isPinned ? 'unpinTab' : 'pinTab'),
  click: (item) => {
@@ -1164,7 +1164,7 @@ function mainTemplateInit (nodeProps, frame) {
  })
  }
 
- if (frame.get('location') === 'about:bookmarks') {
+ if (frame.get('location') === config.aboutPages.bookmarks) {
  template.push(
  CommonMenu.separatorMenuItem,
  addBookmarkMenuItem('addBookmark', {
@@ -1353,7 +1353,7 @@ function onBackButtonHistoryMenu (activeFrame, history, rect) {
  {
  label: locale.translation('showAllHistory'),
  click: (e, focusedWindow) => {
- windowActions.newFrame({ location: 'about:history' })
+ windowActions.newFrame({ location: config.aboutPages.history })
  windowActions.setContextMenuDetail()
  }
  })
@@ -1396,7 +1396,7 @@ function onForwardButtonHistoryMenu (activeFrame, history, rect) {
  {
  label: locale.translation('showAllHistory'),
  click: (e, focusedWindow) => {
- windowActions.newFrame({ location: 'about:history' })
+ windowActions.newFrame({ location: config.aboutPages.history })
  windowActions.setContextMenuDetail()
  }
  })

js/data/searchProviders.js

@@ -30,6 +30,7 @@ module.exports = { "providers" :
  },
  {
  "name" : "GitHub",
+ "base" : "https://github.com/search",
  "image" : "https://assets-cdn.github.com/favicon.ico",
  "search" : "https://github.com/search?q={searchTerms}",
  "shortcut" : ":gh"
@@ -44,12 +45,14 @@ module.exports = { "providers" :
  },
  {
  "name" : "Stack Overflow",
+ "base" : "https://stackoverflow.com/search",
  "image" : "https://cdn.sstatic.net/sites/stackoverflow/img/favicon.ico",
  "search" : "https://stackoverflow.com/search?q={searchTerms}",
  "shortcut" : ":s"
  },
  {
  "name" : "Mozilla Developer Network (MDN)",
+ "base": "https://developer.mozilla.org/search",
  "image" : "https://developer.cdn.mozilla.net/static/img/favicon32.png",
  "search" : "https://developer.mozilla.org/search?q={searchTerms}",
  "shortcut" : ":m"
@@ -86,6 +89,7 @@ module.exports = { "providers" :
  },
  {
  "name" : "StartPage",
+ "base" : "https://www.startpage.com",
  "image" : "https://www.startpage.com/graphics/favicon/sp-favicon-16x16.png",
  "search" : "https://www.startpage.com/do/dsearch?query={searchTerms}&cat=web&pl=opensearch",
  "autocomplete": "https://www.startpage.com/cgi-bin/csuggest?query={searchTerms}&limit=10&format=json",

js/lib/appUrlUtil.js

@@ -6,9 +6,6 @@ const Immutable = require('immutable')
 const path = require('path')
 const UrlUtil = require('./urlutil')
 const config = require('../constants/config')
-const getSetting = require('../settings').getSetting
-const settings = require('../constants/settings')
-const searchProviders = require('../data/searchProviders').providers
 
 module.exports.fileUrl = (str) => {
  var pathName = path.resolve(str).replace(/\\/g, '/')
@@ -155,26 +152,28 @@ module.exports.navigatableTypes = ['http:', 'https:', 'about:', 'chrome:', 'chro
 /**
  * Grabs the url of the new tab
  */
- // module.exports.newFrameUrl = function () {
- // let newTabMode = getSetting(settings.NEWTAB_MODE)
- // let defaultUrl
- // switch (newTabMode) {
- // case 'newTabPage':
- // defaultUrl = 'about:newtab'
- // break
- // case 'homePage':
- // defaultUrl = getSetting(settings.HOMEPAGE)
- // break
- // case 'defaultSearchEngine':
- // let defaultSearchEngine = getSetting(settings.DEFAULT_SEARCH_ENGINE)
- // let defaultSearchEngineSettings = searchProviders.filter(engine => {
- // return engine.name === defaultSearchEngine
- // })
- // defaultUrl = defaultSearchEngineSettings[0].base
- // break
- // default:
- // defaultUrl = ''
- // break
- // }
- // return defaultUrl
- // }
+module.exports.newFrameUrl = function () {
+ const {getSetting} = require('../settings')
+ const settings = require('../constants/settings')
+ const settingValue = getSetting(settings.NEWTAB_MODE)
+ const {newTabMode} = require('../../app/common/constants/settingsEnums')
+
+ switch (settingValue) {
+ case newTabMode.NEW_TAB_PAGE:
+ return config.aboutPages.newtab
+
+ case newTabMode.HOMEPAGE:
+ return getSetting(settings.HOMEPAGE) || config.aboutPages.blank
+
+ case newTabMode.DEFAULT_SEARCH_ENGINE:
+ const searchProviders = require('../data/searchProviders').providers
+ const defaultSearchEngine = getSetting(settings.DEFAULT_SEARCH_ENGINE)
+ const defaultSearchEngineSettings = searchProviders.filter(engine => {
+ return engine.name === defaultSearchEngine
+ })
+ return defaultSearchEngineSettings[0].base
+
+ default:
+ return config.aboutPages.blank
+ }
+}

js/settings.js

@@ -6,7 +6,7 @@ const appConfig = require('./constants/appConfig')
 const Immutable = require('immutable')
 const settings = require('./constants/settings')
 const {passwordManagers, defaultPasswordManager, extensionIds, displayNames} = require('./constants/passwordManagers')
-const bookmarksToolbarMode = require('../app/common/constants/bookmarksToolbarMode')
+const {bookmarksToolbarMode} = require('../app/common/constants/settingsEnums')
 
 const passwordManagerDefault = (settingKey, settingsCollection) => {
  const onePasswordEnabled = resolveValue(settings.ONE_PASSWORD_ENABLED, settingsCollection) === true