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/frame.js

@@ -33,7 +33,6 @@ const currentWindow = require('../../app/renderer/currentWindow')
 const windowStore = require('../stores/windowStore')
 const appStoreRenderer = require('../stores/appStoreRenderer')
 const siteSettings = require('../state/siteSettings')
-const addSiteDebounced = debounce((frame) => appActions.addSite(siteUtil.getDetailFromFrame(frame)), 1000)
 
 const WEBRTC_DEFAULT = 'default'
 const WEBRTC_DISABLE_NON_PROXY = 'disable_non_proxied_udp'
@@ -78,6 +77,10 @@ class Frame extends ImmutableComponent {
  return isIntermediateAboutPage(getBaseUrl(this.props.location))
  }
 
+ /**
+ * Send data critical for the given about page via IPC.
+ * The page receiving the data typically uses it in component state.
+ */
  updateAboutDetails () {
  let location = getBaseUrl(this.props.location)
  if (location === 'about:preferences') {
@@ -951,8 +954,7 @@ class Frame extends ImmutableComponent {
  const isError = this.props.aboutDetails && this.props.aboutDetails.get('errorCode')
  if (!this.props.isPrivate && this.props.provisionalLocation === this.props.location && (protocol === 'http:' || protocol === 'https:') && !isError && savePage) {
  // Register the site for recent history for navigation bar
- addSiteDebounced(this.frame)
- // appActions.addSite(siteUtil.getDetailFromFrame(this.frame))
+ appActions.addSite(siteUtil.getDetailFromFrame(this.frame))
  }
 
  const hack = siteHacks[parsedUrl.hostname]

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')
@@ -88,23 +84,7 @@ class Tabs extends ImmutableComponent {
  }
 
  newTab () {
- const newTabMode = getSetting(settings.NEWTAB_MODE)
- switch (newTabMode) {
- case 'homePage':
- windowActions.newFrame({location: getSetting(settings.HOMEPAGE)})
- break
- case 'defaultSearchEngine':
- const defaultSearchEngine = getSetting(settings.DEFAULT_SEARCH_ENGINE)
- let defaultSearchEngineSettings = searchProviders.filter(engine => {
- return engine.name === defaultSearchEngine
- })
- windowActions.newFrame({location: defaultSearchEngineSettings[0].base})
- break
- case 'newTabPage':
- default:
- windowActions.newFrame()
- break
- }
+ windowActions.newFrame()
  }
 
  render () {

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/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')

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

js/stores/windowStore.js

@@ -18,8 +18,7 @@ const urlParse = require('url').parse
 const currentWindow = require('../../app/renderer/currentWindow')
 const {tabFromFrame} = require('../state/frameStateUtil')
 const {l10nErrorText} = require('../../app/common/lib/httpUtil')
-const {aboutUrls, getSourceAboutUrl, isIntermediateAboutPage, navigatableTypes} = require('../lib/appUrlUtil')
-const searchProviders = require('../data/searchProviders').providers
+const {aboutUrls, getSourceAboutUrl, isIntermediateAboutPage, navigatableTypes, newFrameUrl} = require('../lib/appUrlUtil')
 const Serializer = require('../dispatcher/serializer')
 
 let windowState = Immutable.fromJS({
@@ -164,8 +163,6 @@ const addToHistory = (frameProps) => {
 }
 
 const newFrame = (frameOpts, openInForeground, insertionIndex, nextKey) => {
- const frames = windowState.get('frames')
-
  if (frameOpts === undefined) {
  frameOpts = {}
  }
@@ -175,11 +172,12 @@ const newFrame = (frameOpts, openInForeground, insertionIndex, nextKey) => {
  openInForeground = true
  }
 
- // let defaultUrl = newFrameUrl()
- frameOpts.location = frameOpts.location || config.defaultUrl
+ // evaluate the location
+ frameOpts.location = frameOpts.location || newFrameUrl()
  if (frameOpts.location && UrlUtil.isURL(frameOpts.location)) {
  frameOpts.location = UrlUtil.getUrlFromInput(frameOpts.location)
  } else {
+ // location is a search
  const defaultURL = windowStore.getState().getIn(['searchDetail', 'searchURL'])
  if (defaultURL) {
  frameOpts.location = defaultURL
@@ -205,6 +203,7 @@ const newFrame = (frameOpts, openInForeground, insertionIndex, nextKey) => {
 
  // Find the closest index to the current frame's index which has
  // a different ancestor frame key.
+ const frames = windowState.get('frames')
  if (insertionIndex === undefined) {
  insertionIndex = FrameStateUtil.findIndexForFrameKey(frames, frameOpts.parentFrameKey)
  if (insertionIndex === -1) {