Skip to content

Commit

Permalink
fix: promise resolved to early when browser initiated in-page navigat…
Browse files Browse the repository at this point in the history
…ion v2 (#39679)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Tomasz Malinowski <tomasz@openfin.co>
  • Loading branch information
trop[bot] and TomaszMa committed Aug 31, 2023
1 parent 81bd816 commit 3f1fee7
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
11 changes: 9 additions & 2 deletions lib/browser/api/web-contents.ts
Expand Up @@ -447,6 +447,7 @@ WebContents.prototype.loadURL = function (url, options) {
};

let navigationStarted = false;
let browserInitiatedInPageNavigation = false;
const navigationListener = (event: Electron.Event, url: string, isSameDocument: boolean, isMainFrame: boolean) => {
if (isMainFrame) {
if (navigationStarted && !isSameDocument) {
Expand All @@ -461,6 +462,7 @@ WebContents.prototype.loadURL = function (url, options) {
// as the routing does not leave the document
return rejectAndCleanup(-3, 'ERR_ABORTED', url);
}
browserInitiatedInPageNavigation = navigationStarted && isSameDocument;
navigationStarted = true;
}
};
Expand All @@ -475,17 +477,22 @@ WebContents.prototype.loadURL = function (url, options) {
// would be more appropriate.
rejectAndCleanup(-2, 'ERR_FAILED', url);
};
const finishListenerWhenUserInitiatedNavigation = () => {
if (!browserInitiatedInPageNavigation) {
finishListener();
}
};
const removeListeners = () => {
this.removeListener('did-finish-load', finishListener);
this.removeListener('did-fail-load', failListener);
this.removeListener('did-navigate-in-page', finishListener);
this.removeListener('did-navigate-in-page', finishListenerWhenUserInitiatedNavigation);
this.removeListener('did-start-navigation', navigationListener);
this.removeListener('did-stop-loading', stopLoadingListener);
this.removeListener('destroyed', stopLoadingListener);
};
this.on('did-finish-load', finishListener);
this.on('did-fail-load', failListener);
this.on('did-navigate-in-page', finishListener);
this.on('did-navigate-in-page', finishListenerWhenUserInitiatedNavigation);
this.on('did-start-navigation', navigationListener);
this.on('did-stop-loading', stopLoadingListener);
this.on('destroyed', stopLoadingListener);
Expand Down
10 changes: 10 additions & 0 deletions spec/api-web-contents-spec.ts
Expand Up @@ -375,6 +375,16 @@ describe('webContents module', () => {
await expect(w.loadURL(w.getURL() + '#foo')).to.eventually.be.fulfilled();
});

it('resolves after browser initiated navigation', async () => {
let finishedLoading = false;
w.webContents.on('did-finish-load', function () {
finishedLoading = true;
});

await w.loadFile(path.join(fixturesPath, 'pages', 'navigate_in_page_and_wait.html'));
expect(finishedLoading).to.be.true();
});

it('rejects when failing to load a file URL', async () => {
await expect(w.loadURL('file:non-existent')).to.eventually.be.rejected()
.and.have.property('code', 'ERR_FILE_NOT_FOUND');
Expand Down
15 changes: 15 additions & 0 deletions spec/fixtures/pages/navigate_in_page_and_wait.html
@@ -0,0 +1,15 @@
<html>
<header>
<script type="text/javascript">
window.history.replaceState(window.location.href, "Sample Title", window.location.href);
// Simulate that we load web page.
let d = new Date();
const endTime = new Date(d.getTime() + (10 * 1000));
while(d.getTime() < endTime) {
d = new Date();
}
</script>
</header>
<body>
</body>
</html>

0 comments on commit 3f1fee7

Please sign in to comment.