fix: move open handling to web-contents.js (#16623)

lib/browser/api/browser-window.js

@@ -3,8 +3,6 @@
 const electron = require('electron')
 const { WebContentsView, TopLevelWindow } = electron
 const { BrowserWindow } = process.atomBinding('window')
-const v8Util = process.atomBinding('v8_util')
-const ipcMain = require('@electron/internal/browser/ipc-main-internal')
 
 Object.setPrototypeOf(BrowserWindow.prototype, TopLevelWindow.prototype)
 
@@ -27,69 +25,6 @@ BrowserWindow.prototype._init = function () {
     nativeSetBounds.call(this, bounds, ...opts)
   }
 
-  // Make new windows requested by links behave like "window.open"
-  this.webContents.on('-new-window', (event, url, frameName, disposition,
-    additionalFeatures, postData,
-    referrer) => {
-    const options = {
-      show: true,
-      width: 800,
-      height: 600
-    }
-    ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN',
-      event, url, referrer, frameName, disposition,
-      options, additionalFeatures, postData)
-  })
-
-  this.webContents.on('-web-contents-created', (event, webContents, url,
-    frameName) => {
-    v8Util.setHiddenValue(webContents, 'url-framename', { url, frameName })
-  })
-
-  // Create a new browser window for the native implementation of
-  // "window.open", used in sandbox and nativeWindowOpen mode
-  this.webContents.on('-add-new-contents', (event, webContents, disposition,
-    userGesture, left, top, width,
-    height) => {
-    const urlFrameName = v8Util.getHiddenValue(webContents, 'url-framename')
-    if ((disposition !== 'foreground-tab' && disposition !== 'new-window' &&
-         disposition !== 'background-tab') || !urlFrameName) {
-      event.preventDefault()
-      return
-    }
-
-    if (webContents.getLastWebPreferences().nodeIntegration === true) {
-      const message =
-        'Enabling Node.js integration in child windows opened with the ' +
-        '"nativeWindowOpen" option will cause memory leaks, please turn off ' +
-        'the "nodeIntegration" option.\\n' +
-        'From 5.x child windows opened with the "nativeWindowOpen" option ' +
-        'will always have Node.js integration disabled.\\n' +
-        'See https://github.com/electron/electron/pull/15076 for more.'
-      // console is only available after DOM is created.
-      const printWarning = () => this.webContents.executeJavaScript(`console.warn('${message}')`)
-      if (this.webContents.isDomReady()) {
-        printWarning()
-      } else {
-        this.webContents.once('dom-ready', printWarning)
-      }
-    }
-
-    const { url, frameName } = urlFrameName
-    v8Util.deleteHiddenValue(webContents, 'url-framename')
-    const options = {
-      show: true,
-      x: left,
-      y: top,
-      width: width || 800,
-      height: height || 600,
-      webContents: webContents
-    }
-    const referrer = { url: '', policy: 'default' }
-    ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN',
-      event, url, referrer, frameName, disposition, options)
-  })
-
   // window.resizeTo(...)
   // window.moveTo(...)
   this.webContents.on('move', (event, size) => {

lib/browser/api/web-contents.js

@@ -5,6 +5,7 @@ const { EventEmitter } = require('events')
 const electron = require('electron')
 const path = require('path')
 const url = require('url')
+const v8Util = process.atomBinding('v8_util')
 const { app, ipcMain, session, NavigationController, deprecate } = electron
 
 const ipcMainInternal = require('@electron/internal/browser/ipc-main-internal')
@@ -384,6 +385,72 @@ WebContents.prototype._init = function () {
     this.reload()
   })
 
+  // Handle window.open for BrowserWindow and BrowserView.
+  if (['browserView', 'window'].includes(this.getType())) {
+    // Make new windows requested by links behave like "window.open"
+    this.webContents.on('-new-window', (event, url, frameName, disposition,
+      additionalFeatures, postData,
+      referrer) => {
+      const options = {
+        show: true,
+        width: 800,
+        height: 600
+      }
+      ipcMainInternal.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN',
+        event, url, referrer, frameName, disposition,
+        options, additionalFeatures, postData)
+    })
+
+    this.webContents.on('-web-contents-created', (event, webContents, url,
+      frameName) => {
+      v8Util.setHiddenValue(webContents, 'url-framename', { url, frameName })
+    })
+
+    // Create a new browser window for the native implementation of
+    // "window.open", used in sandbox and nativeWindowOpen mode
+    this.webContents.on('-add-new-contents', (event, webContents, disposition,
+      userGesture, left, top, width,
+      height) => {
+      const urlFrameName = v8Util.getHiddenValue(webContents, 'url-framename')
+      if ((disposition !== 'foreground-tab' && disposition !== 'new-window' &&
+           disposition !== 'background-tab') || !urlFrameName) {
+        event.preventDefault()
+        return
+      }
+
+      if (webContents.getLastWebPreferences().nodeIntegration === true) {
+        const message =
+          'Enabling Node.js integration in child windows opened with the ' +
+          '"nativeWindowOpen" option will cause memory leaks, please turn off ' +
+          'the "nodeIntegration" option.\\n' +
+          'From 5.x child windows opened with the "nativeWindowOpen" option ' +
+          'will always have Node.js integration disabled.\\n' +
+          'See https://github.com/electron/electron/pull/15076 for more.'
+        // console is only available after DOM is created.
+        const printWarning = () => this.webContents.executeJavaScript(`console.warn('${message}')`)
+        if (this.webContents.isDomReady()) {
+          printWarning()
+        } else {
+          this.webContents.once('dom-ready', printWarning)
+        }
+      }
+
+      const { url, frameName } = urlFrameName
+      v8Util.deleteHiddenValue(webContents, 'url-framename')
+      const options = {
+        show: true,
+        x: left,
+        y: top,
+        width: width || 800,
+        height: height || 600,
+        webContents
+      }
+      const referrer = { url: '', policy: 'default' }
+      ipcMainInternal.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN',
+        event, url, referrer, frameName, disposition, options)
+    })
+  }
+
   app.emit('web-contents-created', {}, this)
 }
 

spec/api-browser-view-spec.js

@@ -1,5 +1,6 @@
 'use strict'
 
+const assert = require('assert')
 const chai = require('chai')
 const ChildProcess = require('child_process')
 const dirtyChai = require('dirty-chai')
@@ -14,6 +15,8 @@ const { expect } = chai
 chai.use(dirtyChai)
 
 describe('BrowserView module', () => {
+  const fixtures = path.resolve(__dirname, 'fixtures')
+
   let w = null
   let view = null
 
@@ -178,11 +181,26 @@ describe('BrowserView module', () => {
 
   describe('new BrowserView()', () => {
     it('does not crash on exit', async () => {
-      const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-browserview.js')
+      const appPath = path.join(fixtures, 'api', 'leak-exit-browserview.js')
       const electronPath = remote.getGlobal('process').execPath
       const appProcess = ChildProcess.spawn(electronPath, [appPath])
       const [code] = await emittedOnce(appProcess, 'close')
       expect(code).to.equal(0)
     })
   })
+
+  describe('window.open()', () => {
+    it('works in BrowserView', (done) => {
+      view = new BrowserView()
+      w.setBrowserView(view)
+      view.webContents.once('new-window', (e, url, frameName, disposition, options, additionalFeatures) => {
+        e.preventDefault()
+        assert.strictEqual(url, 'http://host/')
+        assert.strictEqual(frameName, 'host')
+        assert.strictEqual(additionalFeatures[0], 'this-is-not-a-standard-feature')
+        done()
+      })
+      view.webContents.loadFile(path.join(fixtures, 'pages', 'window-open.html'))
+    })
+  })
 })