sindresorhus/electron-context-menu

Support `BrowserView`

sindresorhus opened this issue · 8 comments

sindresorhus commented

https://www.electronjs.org/docs/api/browser-view

This issue requires you to have advanced JS and Electron knowledge. It's not an easy issue.

josephernest commented

(I removed this post from the other closed issue, no longer needed)

Reproducible test:

main.js:

const {app, BrowserWindow, BrowserView} = require('electron');
const contextMenu = require('electron-context-menu');

contextMenu({prepend: (defaultActions, params, browserWindow) => [{ label: 'Test', visible: true }]});

app.on('ready', () => {
    browserWindow = new BrowserWindow();
    browserWindow.webContents.loadURL('file://' + __dirname + '/index.html');

    bv = new BrowserView();
    bv.setBounds({ x: 0, y: 100, width: 800, height: 400}); 
    bv.webContents.loadURL('https://example.com');
    browserWindow.setBrowserView(bv);
});

index.html

<html>
<body>
Hello world, right click on me! Context menu works!
</body>
</html>

image

Right-clicking on Hello world, right click on me! works.

Expected behaviour: context menu should also appear when right-clicking on Example domain

josephernest commented

I think we have to (see https://www.electronjs.org/docs/api/browser-window#wingetbrowserviews-experimental):

  • loop over each item of win.getBrowserViews(), as bv
  • we can access the webContents with bv.webContents

I don't know if we can keep (https://github.com/sindresorhus/electron-context-menu/blob/master/index.js#L7):

const webContents = win => win.webContents || (win.getWebContentsId && electron.remote.webContents.fromId(win.getWebContentsId()));

because there can be multiple webContents in the same window, for example:

  • browserWindow.webContents
  • browserWindow.getBrowserViews()[0].webContents
  • browserWindow.getBrowserViews()[1].webContents
  • etc.
josephernest commented

Do you think we should do like this @sindresorhus?

The problem is that your line

const webContents = ...

uses only one fixed webContent.

Question: is it possible to run the init of your library, applied to a specific webContents? Something like:

const contextMenu = require('electron-context-menu');
contextMenu.addToWebContent(browserview2.webContents)

?

sindresorhus commented

I don't have any answers. I don't use the browser views feature.

sindresorhus commented

Question: is it possible to run the init of your library, applied to a specific webContents? Something like:

The window option supports web contents, which you can get from the browser view.

josephernest commented

Good news @sindresorhus ! This works (no HTML file required here to do the test):

const {app, BrowserWindow, BrowserView} = require('electron');

const contextMenu = require('electron-context-menu');

app.on('ready', () => {
    browserWindow = new BrowserWindow();
    browserWindow.webContents.loadURL('data:text/html,<html><body>Hello world, right click on me! Context menu works!</body></html>');

    const bv = new BrowserView();
    browserWindow.setBrowserView(bv);
    bv.setBounds({ x: 0, y: 100, width: 800, height: 400}); 
    bv.webContents.loadURL('https://electronjs.org');

    bv.webContents.on('did-finish-load', () => {
        console.log('hello');
        contextMenu({window: bv, prepend: (defaultActions, params, browserWindow) => [{ label: 'Test', visible: true }]});
    });
});

I passed the BrowserView via

    contextMenu({window: bv, ...

Only problem: error dialog box:

win.once is not a function

when this is executed: https://github.com/sindresorhus/electron-context-menu/blob/master/index.js#L340

This is normal because this does not exist for browserView.

... but the context menu is finally working anyway :)

Solution:

Do you think you could add an option browserview: true in options = { } to bypass https://github.com/sindresorhus/electron-context-menu/blob/master/index.js#L340 and the following lines?

Or even better:

add a test if typeof win.once !== 'undefined' or something like that here:

https://github.com/sindresorhus/electron-context-menu/blob/master/index.js#L340

sindresorhus commented

add a test if typeof win.once !== 'undefined' or something like that here:

I'm happy to take a PR for that.

josephernest commented

Here it is @sindresorhus #127

  • 👍1