Skip to content

Commit

Permalink
fix: emit will-navigate for links in chrome: pages (#40524)
Browse files Browse the repository at this point in the history
* fix: emit will-navigate for links in chrome: pages

Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

* test: will-navigate emitted from chrome: pages

Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>

* Update shell/browser/electron_navigation_throttle.cc

Co-authored-by: Charles Kerr <charles@charleskerr.com>

Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Samuel Maddock <smaddock@slack-corp.com>
Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>
  • Loading branch information
3 people committed Nov 15, 2023
1 parent 03acfc9 commit 1eb0d77
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
19 changes: 17 additions & 2 deletions shell/browser/electron_navigation_throttle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

#include "shell/browser/electron_navigation_throttle.h"

#include "content/browser/renderer_host/render_frame_host_impl.h" // nogncheck
#include "content/public/browser/navigation_handle.h"
#include "shell/browser/api/electron_api_web_contents.h"
#include "shell/browser/javascript_environment.h"
#include "ui/base/page_transition_types.h"

namespace electron {

Expand Down Expand Up @@ -37,11 +39,24 @@ ElectronNavigationThrottle::WillStartRequest() {
return PROCEED;
}

if (handle->IsRendererInitiated() &&
bool is_renderer_initiated = handle->IsRendererInitiated();

// Chrome's internal pages always mark navigation as browser-initiated. Let's
// ignore that.
// https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/navigator.cc;l=883-892;drc=0c8ffbe78dc0ef2047849e45cefb3f621043e956
if (!is_renderer_initiated &&
ui::PageTransitionIsWebTriggerable(handle->GetPageTransition())) {
auto* render_frame_host = contents->GetPrimaryMainFrame();
auto* rfh_impl =
static_cast<content::RenderFrameHostImpl*>(render_frame_host);
is_renderer_initiated = rfh_impl && rfh_impl->web_ui();
}

if (is_renderer_initiated &&
api_contents->EmitNavigationEvent("will-frame-navigate", handle)) {
return CANCEL;
}
if (handle->IsRendererInitiated() && handle->IsInMainFrame() &&
if (is_renderer_initiated && handle->IsInMainFrame() &&
api_contents->EmitNavigationEvent("will-navigate", handle)) {
return CANCEL;
}
Expand Down
20 changes: 20 additions & 0 deletions spec/api-browser-window-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,26 @@ describe('BrowserWindow module', () => {
expect(initiator).not.to.be.undefined();
expect(initiator).to.equal(subframe);
});

it('is triggered when navigating from chrome: to http:', async () => {
let hasEmittedWillNavigate = false;
const willNavigatePromise = new Promise((resolve) => {
w.webContents.once('will-navigate', e => {
e.preventDefault();
hasEmittedWillNavigate = true;
resolve(e.url);
});
});
await w.loadURL('chrome://gpu');

// shouldn't emit for browser-initiated request via loadURL
expect(hasEmittedWillNavigate).to.equal(false);

w.webContents.executeJavaScript(`location.href = ${JSON.stringify(url)}`);
const navigatedTo = await willNavigatePromise;
expect(navigatedTo).to.equal(url + '/');
expect(w.webContents.getURL()).to.equal('chrome://gpu/');
});
});

describe('will-frame-navigate event', () => {
Expand Down

0 comments on commit 1eb0d77

Please sign in to comment.