Use untitled and loading for opener pages

Fix #6495 Auditors: @bsclifton This feels a lot better especially with the loading icon being used now
brave · Jan 6, 2017 · cfa8388 · cfa8388 · bsclifton · Jan 6, 2017
1 parent b1def4d
commit cfa8388
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 6 deletions.
diff --git a/app/extensions/brave/about-blank.html b/app/extensions/brave/about-blank.html
@@ -7,10 +7,10 @@
     <meta charset="utf-8">
     <meta name="availableLanguages" content="">
     <meta name="defaultLanguage" content="en-US">
-    <title data-l10n-id="aboutBlank"></title>
+    <title data-l10n-id="aboutBlankTitle"></title>
     <script src="ext/l20n.min.js"></script>
     <script src="gen/aboutPages.entry.js" async></script>
-    <link rel="localization" href="locales/{locale}/preferences.properties">
+    <link rel="localization" href="locales/{locale}/common.properties">
   </head>
   <body />
 </html>
diff --git a/app/extensions/brave/locales/en-US/common.properties b/app/extensions/brave/locales/en-US/common.properties
@@ -1,2 +1,3 @@
 enableWidevineSubtext=Google Widevine is a piece of Digital Rights Management (DRM) code that we at Brave Software do not own and cannot inspect. The Google Widevine code is loaded from Google servers, not from our servers. It is loaded only when you enable this option. We discourage the use of DRM, but we respect user choice and acknowledge that some Brave users would like to use services that require it.
 enableWidevineSubtext2=By installing this extension, you are agreeing to the Google Widevine Terms of Use. You agree that Brave is not responsible for any damages or losses in connection with your use of Google Widevine.
+aboutBlankTitle=Untitled
diff --git a/app/locale.js b/app/locale.js
@@ -171,6 +171,7 @@ var rendererIdentifiers = function () {
     'ignoreSpelling',
     'lookupSelection',
     // Other identifiers
+    'aboutBlankTitle',
     'urlCopied',
     'autoHideMenuBar',
     'unexpectedErrorWindowReload',
@@ -353,10 +354,12 @@ exports.init = function (language) {
   const propertyFiles = []
   const appendLangProperties = function (lang) {
     // Property files to parse (only ones containing menu specific identifiers)
-    propertyFiles.push(path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'menu.properties'),
+    propertyFiles.push(
+      path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'menu.properties'),
       path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'app.properties'),
       path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'error.properties'),
-      path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'passwords.properties'))
+      path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'passwords.properties'),
+      path.join(__dirname, 'extensions', 'brave', 'locales', lang, 'common.properties'))
   }
 
   appendLangProperties(lang)

diff --git a/js/components/tab.js b/js/components/tab.js
@@ -7,6 +7,7 @@ const React = require('react')
 const ImmutableComponent = require('./immutableComponent')
 
 const windowActions = require('../actions/windowActions')
+const locale = require('../l10n')
 const dragTypes = require('../constants/dragTypes')
 const messages = require('../constants/messages')
 const cx = require('../lib/classSet')
@@ -71,6 +72,13 @@ class Tab extends ImmutableComponent {
   }
 
   get displayValue () {
+    // For renderer initiated navigations, make sure we show Untitled
+    // until we know what we're loading.  We should probably do this for
+    // all about: pages that we already know the title for so we don't have
+    // to wait for the title to be parsed.
+    if (this.props.tab.get('location') === 'about:blank') {
+      return locale.translation('aboutBlankTitle')
+    }
     // YouTube tries to change the title to add a play icon when
     // there is audio. Since we have our own audio indicator we get
     // rid of it.
@@ -107,7 +115,8 @@ class Tab extends ImmutableComponent {
 
   get loading () {
     return this.frame &&
-    this.props.tab.get('loading') &&
+    (this.props.tab.get('loading') ||
+     this.props.tab.get('location') === 'about:blank') &&
     (!this.props.tab.get('provisionalLocation') ||
     !this.props.tab.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/'))
   }
@@ -178,7 +187,7 @@ class Tab extends ImmutableComponent {
       playIcon = true
     }
 
-    const locationHasFavicon = this.props.tab.get('location') !== 'about:newtab' && this.props.tab.get('location') !== 'about:blank'
+    const locationHasFavicon = this.props.tab.get('location') !== 'about:newtab'
 
     return <div
       className={cx({

diff --git a/test/components/tabTest.js b/test/components/tabTest.js
@@ -2,6 +2,7 @@
 
 const Brave = require('../lib/brave')
 const messages = require('../../js/constants/messages')
+const assert = require('assert')
 const settings = require('../../js/constants/settings')
 const {urlInput, backButton, forwardButton, activeTabTitle, activeTabFavicon, newFrameButton} = require('../lib/selectors')
 
@@ -330,4 +331,19 @@ describe('tab tests', function () {
         .waitForExist(activeTabFavicon, 1000, true)
     })
   })
+
+  describe('about:blank tab', function () {
+    Brave.beforeAll(this)
+    before(function * () {
+      yield setup(this.app.client)
+    })
+
+    it('has untitled text right away', function * () {
+      yield this.app.client
+        .ipcSend(messages.SHORTCUT_NEW_FRAME, 'about:blank', { openInForeground: false })
+        .waitForVisible('.tab[data-frame-key="2"]')
+        // This should not be converted to a waitUntil
+        .getText('.tab[data-frame-key="2"]').then((val) => assert.equal(val, 'Untitled'))
+    })
+  })
 })