New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nativeWindowOpen from <webview> tags #9568

Merged
merged 20 commits into from May 26, 2017

Conversation

Projects
None yet
4 participants
@kevinsawicki
Contributor

kevinsawicki commented May 23, 2017

Support setting webpreferences="nativeWindowOpen=1" on <webview> tags.

  • Don't crash when using nativeWindowOpen on a <webview> tag
  • Return null from native window.open when called from a <webview> tag that does not have allowpopups set
  • Add more specs

Refs #8963
Fixes #9556

@kevinsawicki kevinsawicki changed the title from [WIP] Support nativeWindowOpen from <weview> tags to Support nativeWindowOpen from <weview> tags May 24, 2017

@kevinsawicki kevinsawicki changed the title from Support nativeWindowOpen from <weview> tags to Support nativeWindowOpen from <webview> tags May 24, 2017

@kevinsawicki kevinsawicki merged commit 8ae36c2 into master May 26, 2017

7 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
electron-linux-arm Build #6782225 succeeded in 107s
Details
electron-linux-ia32 Build #6782226 succeeded in 104s
Details
electron-linux-x64 Build #6782227 succeeded in 204s
Details
electron-mas-x64 Build #4338 succeeded in 10 min
Details
electron-osx-x64 Build #4342 succeeded in 9 min 58 sec
Details
electron-win-ia32 Build #3311 succeeded in 10 min
Details
electron-win-x64 Build #3287 succeeded in 22 min
Details

@kevinsawicki kevinsawicki deleted the webview-native-window-open branch May 26, 2017

@snene

This comment has been minimized.

snene commented May 30, 2017

@kevinsawicki Thanks, I noticed that this change is merged and is available as part of Electron beta 1.7.2. I am currently trying it out, but not working for me as expected.

I just wanted to understand the exact steps that are needed if I want to call native window.open and window.document.write from the pages loaded in webview. Here is what I understand:

  1. Create the BrowserWindow with nativeWindowOpen set to true as follows:
    win = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
    nativeWindowOpen: true
    }
    })
  2. The BrowserWindow will load a html page with a webview with nativeWindowOpen set to true as follows:

My questions are:

  1. Is it still needed to handle new-window event in the BrowserWindow and return the newly created window as event.newGuest?
  2. When the guest page calls window.open, does it still get BrowserWindowProxy as return object? I am assuming not, because I want to call window.open and then window.document.write on that window.

Please refer to my issue #9515 for the sample code of what I want to do. Thanks a lot for your continued support!

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 30, 2017

Is it still needed to handle new-window event in the BrowserWindow and return the newly created window as event.newGuest?

You are not required to override the newGuest from the new-window event unless you want to customize the created window.

When the guest page calls window.open, does it still get BrowserWindowProxy as return object?

It returns a regular Window object similar to Chrome

@snene

This comment has been minimized.

snene commented May 30, 2017

Thanks for the quick reply @kevinsawicki ! So it's ok if I don't handle new-window event and create a new window? It should just work! But I need "allowpopups" to be set on the webview, correct?

I am asking these questions because I am trying it out and still not working for me.

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 30, 2017

So it's ok if I don't handle new-window event and create a new window?

Yes, it should be fine, the docs show it just as an example for creating a truly modal window.

But I need "allowpopups" to be set on the webview, correct?

Yes, window.open will return null unless allowpopups is set.

I am asking these questions because I am trying it out and still not working for me.

Can you post some example code that isn't working for you? Thanks.

@snene

This comment has been minimized.

snene commented May 30, 2017

Thanks! Actually, my original example code is in issue #9515

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 30, 2017

Thanks! Actually, my original example code is in issue #9515

Could you post your current code that is still failing for you on 1.7.2?

@snene

This comment has been minimized.

snene commented May 31, 2017

Sample code is as follows:
Main app:

const {app, BrowserWindow, ipcMain} = require("electron")

let win = null

app.on('ready', () => {
  win = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      nativeWindowOpen: true
    }
  });
 win.loadURL('./MyWindow.html');
  win.on("closed", () => {
    win = null
});

MyWindow.html:

<html>
  <body>
        <div class="mainView">
            <webview id="webview" style="height:100%" src="about:blank"  webpreferences="nodeIntegration=no, nativeWindowOpen=true" allowpopups></webview>
        </div>
    </div>
    <script src="../src/js/ClientShell.js"></script>
  </body>
</html>

ClientShell.js

const BrowserWindowR = require('electron').remote.BrowserWindow;

 webview.addEventListener('dom-ready', () => {
    webview.loadURL('Print.html');
});

Print.html has code which open a window like following:

var printWin = window.open("", "_blank");
printWin.document.open();
printWin.document.write(<some html text>);
printWin.document.close();

I see that the printWin.document.open still fails with the error "TypeError: Cannot read property 'open' of undefined". This is happening because window.open is still returning BrowserWindowProxy and so printWin is set to type BrowserWindowProxy which does not have "document" defined on it.

@kevinsawicki can you please look into it?

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 31, 2017

<webview id="webview" style="height:100%" src="about:blank" webpreferences="nodeIntegration=no, nativeWindowOpen=true" allowpopups></webview>

nativeWindowOpen needs to be yes or 1 to be enabled. Can you try:

 <webview id="webview" style="height:100%" src="about:blank"  webpreferences="nodeIntegration=no, nativeWindowOpen=yes" allowpopups></webview>
@snene

This comment has been minimized.

snene commented May 31, 2017

Yesssss, that worked! This is awesome. It just solved one of our biggest issue. Thanks a lot @kevinsawicki for all the help and support you provided! Do you know when will this be available in the official release of Electron?

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 31, 2017

Yesssss, that worked!

Glad to hear it 👍

Do you know when will this be available in the official release of Electron?

I don't have a firm timeline at this point, I would think if all goes well with people testing the beta release it could hit stable in a week or so.

@snene

This comment has been minimized.

snene commented Jun 5, 2017

@kevinsawicki I understand that using "nativeWindowOpen=yes" for a webview needs "allowpopups" to be set on the webview. But is it still possible to override the functionality by using 'new-window' handler and calling preventDefault. I added the following code to the BrowserWindow:

  webview.addEventListener('new-window', (evt) => {
    if (<check for some condition>) {
         evt.preventDefault();
     }
  });

But still a new popup window is created even if evt.preventDefault(); is called.

Is it possible to stop popup window in certain cases? If yes, how? Thanks a million!

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented Jun 5, 2017

Is it possible to stop popup window in certain cases? If yes, how? Thanks a million!

You need to listen to this event from the main process, webview.addEventListener does not support preventing the default for events since they are dispatched through the main process. You can listen to <webview> events from a renderer process but modifying the event parameters or preventing the default is not possible.

Can you try this instead from the main process:

app.on('web-contents-created', (event, contents) => {
  if (contents.getType() === 'webview') {
    contents.on('new-window', (event) => {
      console.log('blocking!')
      event.preventDefault()
    })
  }
})
@snene

This comment has been minimized.

snene commented Jun 5, 2017

Thanks a lot @kevinsawicki Will try and let you know!

@snene

This comment has been minimized.

snene commented Jun 6, 2017

Thanks a lot @kevinsawicki I tried and it works! Thanks again!

@elite28

This comment has been minimized.

elite28 commented Jul 28, 2017

@kevinsawicki setting webpreferences="nativeWindowOpen=1" on <webview> tag, it can open a new window for me, but when the new window excute js after body onload, like get data from opener, the opener is null..

@mkazlauskas

This comment has been minimized.

mkazlauskas commented Aug 22, 2017

The 'new-window' event seems to only be firing with 'nativeWindowOpen=true', and not '1' or 'yes'. Using Electron 1.7.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment