Follow-up fixes to #3857

Fix #3928 Fix #3927 Auditors: @bridiver Test Plan: Same as #3857 but also make sure that entering a URL like nytimes.com in the URL bar does not show a blank location before the page is done loading.
brave · Sep 13, 2016 · 3ff17ae · bridiver · Sep 13, 2016 · bridiver
1 parent 58dbabc
commit 3ff17ae
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 8 deletions.
diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js
@@ -72,14 +72,23 @@ const windowActions = {
         location
       }, true)
     } else {
-      dispatch({
-        actionType: WindowConstants.WINDOW_SET_URL,
-        location,
-        key: frame.get('key')
-      })
+      this.setUrl(location, frame.get('key'))
     }
   },
 
+  /**
+   * Dispatches a message to the store to set the new URL.
+   * @param {string} location
+   * @param {number} key
+   */
+  setUrl: function (location, key) {
+    dispatch({
+      actionType: WindowConstants.WINDOW_SET_URL,
+      location,
+      key
+    })
+  },
+
   /**
    * Dispatches a message to the store to let it know a page has been navigated.
    *

diff --git a/js/components/frame.js b/js/components/frame.js
@@ -23,7 +23,7 @@ const debounce = require('../lib/debounce.js')
 const getSetting = require('../settings').getSetting
 const config = require('../constants/config')
 const settings = require('../constants/settings')
-const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
+const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getSourceAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
 const { isFrameError } = require('../lib/errorUtil')
 const locale = require('../l10n')
 const appConfig = require('../constants/appConfig')
@@ -852,6 +852,12 @@ class Frame extends ImmutableComponent {
         })
         windowActions.loadUrl(this.frame, 'about:error')
         appActions.removeSite(siteUtil.getDetailFromFrame(this.frame))
+      } else {
+        const currentLocation = this.webview.getURL()
+        if (currentLocation !== e.validatedURL) {
+          windowActions.setUrl(isTargetAboutUrl(currentLocation)
+            ? getSourceAboutUrl(currentLocation) : currentLocation, this.props.frameKey)
+        }
       }
     }
     this.webview.addEventListener('load-commit', (e) => {

diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js
@@ -293,6 +293,9 @@ const doAction = (action) => {
         windowState = windowState.mergeIn(tabStatePath(action.key), {
           location: action.location
         })
+        // Show the location for directly-entered URLs before the page finishes
+        // loading
+        updateNavBarInput(action.location, frameStatePath(action.key))
       }
       break
     case WindowConstants.WINDOW_SET_NAVIGATED:

diff --git a/test/components/navigationBarTest.js b/test/components/navigationBarTest.js
@@ -596,12 +596,20 @@ describe('navigationBar', function () {
         yield this.app.client.keys('\uE007')
       })
 
+      it('sets location to new URL immediately', function * () {
+        yield this.app.client
+          .waitUntil(function () {
+            return this.getValue(urlInput).then((val) => {
+              return val === 'https://bayden.com/test/redir/goscript.aspx'
+            })
+          })
+      })
+
       it('clears urlbar if page does not load', function * () {
         yield this.app.client
           .waitUntil(function () {
             return this.getValue(urlInput).then((val) => {
-              console.log('value', val)
-              return val === ''
+              return val.endsWith('/about-newtab.html')
             })
           })
       })