Browse Source

fix: do not mark navigations interupted with same-document navigations as aborted (#18142)

* fix: do not mark navigations interupted with same-document navigations as aborted

* spec: add tests for the loadURL promise
trop[bot] 6 years ago
parent
commit
82bb6d4ccf
2 changed files with 28 additions and 1 deletions
  1. 7 1
      lib/browser/navigation-controller.js
  2. 21 0
      spec/api-browser-window-spec.js

+ 7 - 1
lib/browser/navigation-controller.js

@@ -86,10 +86,16 @@ const NavigationController = (function () {
       let navigationStarted = false
       const navigationListener = (event, url, isSameDocument, isMainFrame, frameProcessId, frameRoutingId, navigationId) => {
         if (isMainFrame) {
-          if (navigationStarted) {
+          if (navigationStarted && !isSameDocument) {
             // the webcontents has started another unrelated navigation in the
             // main frame (probably from the app calling `loadURL` again); reject
             // the promise
+            // We should only consider the request aborted if the "navigation" is
+            // actually navigating and not simply transitioning URL state in the
+            // current context.  E.g. pushState and `location.hash` changes are
+            // considered navigation events but are triggered with isSameDocument.
+            // We can ignore these to allow virtual routing on page load as long
+            // as the routing does not leave the document
             return rejectAndCleanup(-3, 'ERR_ABORTED', url)
           }
           navigationStarted = true

+ 21 - 0
spec/api-browser-window-spec.js

@@ -290,6 +290,27 @@ describe('BrowserWindow module', () => {
       w.loadURL(`data:image/png;base64,${data}`)
     })
 
+    it('should return a promise', () => {
+      const p = w.loadURL('about:blank')
+      expect(p).to.have.property('then')
+    })
+
+    it('should return a promise that resolves', async () => {
+      expect(w.loadURL('about:blank')).to.eventually.be.fulfilled()
+    })
+
+    it('should return a promise that rejects on a load failure', async () => {
+      const data = Buffer.alloc(2 * 1024 * 1024).toString('base64')
+      const p = w.loadURL(`data:image/png;base64,${data}`)
+      await expect(p).to.eventually.be.rejected()
+    })
+
+    it('should return a promise that resolves even if pushState occurs during navigation', async () => {
+      const data = Buffer.alloc(2 * 1024 * 1024).toString('base64')
+      const p = w.loadURL('data:text/html,<script>window.history.pushState({}, "/foo")</script>')
+      await expect(p).to.eventually.be.fulfilled()
+    })
+
     describe('POST navigations', () => {
       afterEach(() => { w.webContents.session.webRequest.onBeforeSendHeaders(null) })