electron / pdf-viewer

Fork of Chrome pdf extension to work as webui page in Electron
51 stars 23 forks source link

Popup Windows supported? #10

Closed icyerasor closed 6 years ago

icyerasor commented 6 years ago

I use plugins:true to display pdfs in the mainWindow

const win = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      plugins: true,
    },
  });

everything works fine. The other usecase is to have the mainWindow open a new popup through something like

    var win = window.open();
    win.location = "https://host.name/file.pdf";

    // this doesn't work at all, as location.replace seems to be not supported by electron?
    // win.location.replace("https://host.name/file.pdf");

The popup opens, and the pdf is offered for download instead of displayed inline. Same link is used for mainWindow-Test / popUp-Url, so its not a content-disposition or something like that. Is the pluins: true not propagated to child-windows?

I tried working around this by intercepting the popup window like this:

mainWindow.webContents.on('new-window', function (evt, url, frameName, disposition, options, additionalFeatures) {
  evt.preventDefault()
  const win = createWindow();
  win.loadURL(url);
  evt.newGuest = win;
  }
);

and creating my own BrowserWindow with plugins: true. That does work, as long as the window.open() is called with an url directly instead of the win.location being updated afterwards. This second part definitly seems more like a general Electron issue? Edit: This second issue seems to be related to nodeIntegration set to false.

timfish commented 6 years ago

In your new-window event, rather than creating an entirely new BrowserWindow have you tried just setting the options? If you don't preventDefault, these are what are used to create the new window.

mainWindow.webContents.on('new-window', function (evt, url, frameName, disposition, options, additionalFeatures) {
 options.webPreferences: {
      plugins: true,
    };
);
icyerasor commented 6 years ago

Hi, that seems like a good idea - I'll try it tomorrow. I think it needs to be options.webPreferences = {.. though, no?

timfish commented 6 years ago

@icyerasor yeah that might work a bit better 😆. It was getting a bit later here.

icyerasor commented 6 years ago

@timfish with the workaround you suggested, it does work; PDFs are displayed in popup windows.

I still have an issue with window.location.replace not working at all in electron, no matter what options I set for nativeWindowOpen, nodeIntegration or sandbox. I guess I have to open a Issue in the main electron project for that.

Thx for the suggestion.

timfish commented 6 years ago

Issues on Github are only for bug reports and feature requests so it'll likely just get closed there. The Electron team use Github issues as their work backlog so questions are best directected to the Slack channel.