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) {