docs: update the example of webContents.setWindowOpenHandler to cla…#49379
docs: update the example of webContents.setWindowOpenHandler to cla…#49379jkleinsc merged 1 commit intoelectron:mainfrom
webContents.setWindowOpenHandler to cla…#49379Conversation
electron-cation
Bot
added
documentation 📓
semver/patch
There was a problem hiding this comment.
Pull request overview
This PR updates the documentation example for webContents.setWindowOpenHandler to properly handle the background-tab disposition case (triggered by middle-click or Ctrl/Cmd+click). The update addresses an issue where Chromium defers the webContents for background tabs, causing it to be undefined in the createWindow callback.
Changes:
- Added a conditional check to manually load the URL for
background-tabdisposition cases - Added an explanatory comment describing why manual URL loading is necessary
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const browserView = new BrowserView(options) | ||
| mainWindow.addBrowserView(browserView) | ||
| browserView.setBounds({ x: 0, y: 0, width: 640, height: 480 }) | ||
| // `background-tab` disposition defers `options.webContents` to become ready here, |
There was a problem hiding this comment.
The comment wording could be clearer. Instead of saying "defers options.webContents to become ready", it would be more accurate to say that options.webContents is undefined for background-tab disposition. Consider rephrasing to: "For background-tab disposition, options.webContents is undefined, so load the URL manually."
| // `background-tab` disposition defers `options.webContents` to become ready here, | |
| // For `background-tab` disposition, options.webContents is undefined, |
erickzhao
added
target/40-x-y
erickzhao
removed
the
target/40-x-y
|
@z0gSh1u Would you be able to rebase your PR on the latest main? That should help pass ci |
![electron-docs-reviewer[bot]](https://avatars.githubusercontent.com/in/243740?s=60&v=4){kind=small}
OK. Latest main merged. |
|
@z0gSh1u we require signed commits before merging: https://www.electronjs.org/docs/latest/development/pull-requests#commit-signing. Can you please sign your commit? |
mlaurencin
added
the
needs-signed-commits
github-actions
Bot
added
the
target/42-x-y
…enHandler` example
OK. I've squashed commits into one, and signed the commit. |
electron-issue-triage
Bot
removed
the
needs-signed-commits
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
Release Notes Persisted
|
|
I have automatically backported this PR to "42-x-y", please check out #50292 |
|
I have automatically backported this PR to "41-x-y", please check out #50293 |
trop
Bot
added
in-flight/42-x-y
in-flight/41-x-y
merged/41-x-y
…rify
background-tabdisposition casesDescription of Change
Resolves #49307 to some extent.
Current
createWindowexample inwebContents.setWindowOpenHandlerdoesn't work well forbackground-tabdisposition (Middle-click, or CmdOrCtrl+Left click). It creates a transparent view that nothing paints on it when usingBrowserView, and causes a main process error forWebContentsView.The root cause is that Chromium would defer the
webContentsofbackground-tabto become ready, so thatoptions.webContentsis undefined. See https://github.com/chromium/chromium/blob/e7662a747d6215ec7c31cd1b226dd38d97fa8baa/content/public/browser/web_contents_delegate.h#L161-L164 and https://github.com/chromium/chromium/blob/main/content/browser/web_contents/web_contents_impl.cc#L5306-L5307, saying thatOpenURLFromTab"returns nullptr if the URL wasn't opened immediately", and the new background tab is not shown immediately.As it's the intended behavior of Chromium, this PR clarifies this point by updating the example, so that the developers can treat this case properly.
A possible concern is that loading the URL by ourselves instead of inheriting the WebContents provided by
optionsmight result in a nullwindow.opener. But after testing on Chrome, Firefox and Safari, all of them don't keep the track ofwindow.openerforbackground-tabdisposition for something like<a href="..." target="_blank" rel="opener">...</a>. So it's okay.Checklist
Release Notes
Notes: Updated the example of webContents.setWindowOpenHandler to clarify background-tab disposition cases