Skip to content
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

Cannot assign to read only property 'onbeforeunload' of object '#<Window>' #1082

Open
entrptaher opened this issue Apr 11, 2017 · 20 comments
Open

Comments

@entrptaher
Copy link
Contributor

entrptaher commented Apr 11, 2017

Any script I Run through medium.com does not work. The site does not load properly. If I open devtools, I can see an error, This error does not happen with normal browsers.
I am using latest nodejs, nightmarejs and electron.

The code I am using simply goes to the site so I can check what went wrong.

var Nightmare = require('nightmare');
var nightmare = Nightmare({
	  openDevTools:{
    	  mode:'detach'
    	},
	  show: true,
	  
	  webPreferences: {
	  	partition: "persist:xyz",
        devTools: true,
        nodeIntegration: false,
        webSecurity: false,
        allowRunningInsecureContent: true
      }})

nightmare.goto('https://medium.com/search?q=nightmarejs&ref=opensearch').then()

Error on version 2.9.0 or above

image

No error on nightmare 2.8.0 (which uses electron-prebuilt)

image

@dzcpy
Copy link

dzcpy commented Apr 27, 2017

Similar problem here. Have you found a solution?

@entrptaher
Copy link
Contributor Author

Using old nightmare is just a temporary solution.

@dzcpy
Copy link

dzcpy commented Apr 27, 2017

I experienced a Cannot assign to read only property 'onunload' of object '#<Window>' error, I somehow got a workaround by opening a new window to run the same code, and it doesn't have any errors. It's so weird.

@entrptaher
Copy link
Contributor Author

Care to share your workaround code?

@dzcpy
Copy link

dzcpy commented Apr 27, 2017

@entrptaher It might not work under all circumstances

const Nightmare = require('nightmare');
require('nightmare-window-manager')(Nightmare);
nightmare
  .windowManager()
  .useragent('Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36')
  .goto('about:blank')
  .evaluate(() => window.open('https://example.com'))
  .waitWindowLoad()
  .currentWindow()
  .then((windows) => {
    console.log(windows.title);
  })

@matusferko
Copy link

+1 same here with 2.10.0

@w01fgang
Copy link

w01fgang commented Jun 8, 2017

I found a workaround

Object.defineProperty(window, 'onbeforeunload', { writable: true });
Object.defineProperty(window, 'onunload', { writable: true });

@bwbuchanan
Copy link

This is a regression introduced by #825 .

The fix is trivial. The preload.js script needs to be modified to:

  // prevent 'onunload' and 'onbeforeunload' from being set
  Object.defineProperties(window, {
    onunload: {
      enumerable: true,
      writable: true,
      value: null,
      set: function() {}
    },
    onbeforeunload: {
      enumerable: true,
      writable: true,
      value: null,
      set: function() {}
    }
  });

Perhaps someone else would be so kind as to put this into a PR? :-)

@w01fgang
Copy link

Unfortunately, this isn't a regression, #825 resolves #824.
Maybe this needs to be documented well. Or someone will add a method that allows changing window properties.
Example:

nightmare.writableWindow(true).goto('http://...')

@bwbuchanan
Copy link

You're right, it's not a regression; it's a bug introduced by #825 when fixing #824.

I think that my fix accomplishes the intent of #825 without breaking pages that try to set onunload / onbeforeunload.

@tomjamescn
Copy link

+1 same here with 2.10.0

@entrptaher
Copy link
Contributor Author

@bwbuchanan, I tried putting that preload.js,
however, it was not fixed in.

@entrptaher
Copy link
Contributor Author

entrptaher commented Aug 18, 2017

I tested your solution, @bwbuchanan. And the result is as you can see.

image

Codes:

var Nightmare = require('nightmare'),
  path = require('path'),
  nightmare = Nightmare({
    show: true,
    webPreferences: {
      preload: path.resolve(__dirname + '/preload.js')
    }
  });

nightmare.goto('http://www.medium.com')
  .url()
  .then((url) => {
    console.log(`Navigated to ${url}`);
  })
  .catch((error) => {
    console.log(error)
  })
window.__nightmare = {};
__nightmare.ipc = require('electron').ipcRenderer;

// prevent 'onunload' and 'onbeforeunload' from being set
Object.defineProperties(window, {
  onunload: {
    enumerable: true,
    writable: true,
    value: null,
    set: function() {}
  },
  onbeforeunload: {
    enumerable: true,
    writable: true,
    value: null,
    set: function() {}
  }
});

This worked which is similar to @w01fgang solution:

Object.defineProperties(window, {
  onunload: {
    writable: true
  },
  onbeforeunload: {
    writable: true
  }
});

Maybe we can turn this into a plugin/action.

@bwbuchanan
Copy link

You must have a different version of something than I do. Try removing "value: null" and replacing with "get: function() { return null; }"

Simply making it writable defeats the purpose of #825

@kireerik
Copy link

I can confirm that setting the below values to true resolves the issue:
https://github.com/segmentio/nightmare/blob/f49265c2d16f66ef86d8fb7bef661b36cbe09073/lib/preload.js#L50

https://github.com/segmentio/nightmare/blob/f49265c2d16f66ef86d8fb7bef661b36cbe09073/lib/preload.js#L55

@matthewmueller Would you accept a pull request with these changes to the preload script?

Would this change brake the other resolved issue previously mentioned above? If so, then I think this shouldn't be part of the default preload script at all. What do you think?

@kvz
Copy link

kvz commented Aug 29, 2018

This happened to me with 3.0.1 just now, just fyi

edit: @kireerik's solution fixed it for me :) thanks!

@sosohime
Copy link

sosohime commented Nov 2, 2018

The way to avoid this problem:
Step 1:
Copy node_modules/nightmare/lib/preload.js somewhere into your project, and replace

https://github.com/segmentio/nightmare/blob/25467e5a1071c05dcbce7f58e137f89c4b8fdd93/lib/preload.js#L47-L58
to

   writable: true,

Step 2: When you init nightmare, webPreferences.preload uses files that have just been modified.

var Nightmare = require('nightmare');
var nightmare = Nightmare({
     webPreferences: {
               ...,
           preload: path.resolve(__dirname, 'mypreload.js')
     }})

@felipemullen
Copy link

This issue is apparently pretty old but I ran into the same problem.

It appears the fix is simple, although modifying preload.js manually is not a viable solution for automated testing

@iblessedi
Copy link

Today I ran into the same issue. It doesn't work! And when I change writable to true I get
Name: TypeError. Message: this.nightmare.writableWindow is not a function

@iblessedi
Copy link

When I downgrade to Nightmare 2.8.1 - I can edit writable. But in the newest version I cannot :(

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

No branches or pull requests