Cannot read property 'firstChild' of null [solved]

Question

Cannot read property 'firstChild' of null [solved]

Chayemor opened this issue 9 years ago · 28 comments

I don't know if this is wcjs-player issue or NW.js issue. It seems that when module is called on new Window, somehow the window objects points to parent window.

Environment

NW.js v0.12.3 (32 bits)
Windows 8
Webchimera.js (player) VLC Build (2.2.1)

Problem
New window is opened from parent window. Webchimera module is loaded but it crashes because its window object points to parent Window.

PARENT

<div id="chimera_container"></div>

...
var gui; // global variable for parent object
var wjs; // global variable for parent object
....
gui = require('nw.gui');
wjs = wjs = require('wcjs-player');
....
video_emit = gui.Window.open('video_emit.html',{
    "window" : {
        "focus": true,
        "toolbar": true,
        "frame": true,
        "width" : 640,
        "height" : 640
    },
    "dependencies": {
        "wcjs-player": "^0.5.6"
    },  
});

video_emit Window

<div id="vid"></div>

....
var wjs; // global variable for new window object
wjs = require('wcjs-player');  // window.require(...) doesn't work either
video_container = new wjs("#vid"); <---- CRASHES **** !!!

In node_modules\wcjs-player\index.js line 82 I added

console.log(window.document);

PARENT CONSOLE OUTPUT

#document
...
<div id="chimera_container"></div>

video_emit CONSOLE OUTPUT

#document
...
<div id="chimera_container"></div>

Somehow when the function in wcjs-player module executes, the window object points to the parent Window always instead of window.video_emit.window . Am I loading the module wrong in the child window?

I know that adding new-instance: true to the gui.Window.open of the parent would fix this, but I need parent and new window to communicate via the global variable usable only if they are on the same render.

Answer 1 · 2016-03-18T20:36:54.000Z

Hey Chayemor,

This isn't a wcjs-player issue, I think this is a node.js issue (not sure if I can call it a nw.js or electron issue per se, even if these platforms are literally the only ones that can create multiple windows). The reason it happens is quite easy to explain, and it's related to all modules, not just wcjs-player.

If you require() a module, it only really gets required once, every time you require it after that, it doesn't matter if you change the window or in what conditions you require it, it will always just return the reference to this module, not reload the module, so if you required it in your first window, wcjs-player gets initialized on that particular window, and as your getting only the reference to it, it will still be directly linked to that first window.

wcjs-player has a partial safe-guard for this, as it removes the module from the cache on page change, but it can't identify new windows in order to clear the cache for them also. The way to do this correctly is by clearing the require cache manually before re-requiring it in the new window, to do this you'd need to do:

// clear wcjs-player from require cache when page changes
var clearModules = [
    "wcjs-player",
    "jquery" // https://github.com/jaruba/wcjs-player/issues/38
];
for (var i in clearModules) {
    if (global.require.cache) {
        for (module in global.require.cache) {
            if (global.require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete global.require.cache[module];
        }
    } else if (require.cache) {
        for (module in require.cache) {
            if (require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete require.cache[module];
        }
    }
}

Answer 2 · 2016-03-18T20:49:49.000Z

Mmm, seems to still be doing it. After clearing cache with your code (console.log of global.require.cache is an empty Object after clear), and then doing

require("wcjs-player");

The same exception occurs because the window.document points to parent document, so it will never find the context when I try to do new () .

Thanks for the help either way.

Answer 3 · 2016-03-18T20:53:31.000Z

try console.log(window.document) before require('wcjs-player')

Answer 4 · 2016-03-18T20:53:53.000Z

Prints out proper window.document.

Answer 5 · 2016-03-18T20:54:20.000Z

that was fast...

Answer 6 · 2016-03-18T20:54:40.000Z

Had tried it already XD.

Answer 7 · 2016-03-18T20:55:21.000Z

let's try to hack it, put everything in a setTimeout(...,1000)

Answer 8 · 2016-03-18T20:59:07.000Z

or maybe in the window.load event..

Answer 9 · 2016-03-18T21:00:13.000Z

Same deal.

First console.log (after setTimeout done after clearRequireCache) is the window's own js file printing window.document . Second console comes from the module, line 81, caused by new wjs("#vid")

Answer 10 · 2016-03-18T21:20:12.000Z

I've had this issue before though, with totally different js files in new windows in nw.js, i was absolutely sure that this require cache was the main reason for it, many have fixed their issues with wcjs-player in new windows by clearing their require caches.

I'm pretty sure I found some hack for it a long time ago, I just can't figure out what it was... for the sake of insanity, let's do something crazy.. copy node_modules/wcjs-player to node_moduels/wcjs-player2 and do require('wcjs-player2') in the new window. :))

Answer 11 · 2016-03-18T21:43:27.000Z

This is insane. Same problem. I even installed require-new module. (link)

var req_new = require('require-new');
wjs = req_new('wcjs-playerdiff');

I went ahead and did NOT require module on my first window. Still happens. How is that even possible?

But if I change the app's package.json file and set video_emit as first window to open up, then it works. But if I open a new window, same problem.

Answer 12 · 2016-03-19T09:12:47.000Z

Don't be amazed, I've even seen errors/console.logs leak to the main window from the .html pages of newly opened windows. Strange thing is, only the first ones leaked, then the rest went to the child window as they should..

Answer 13 · 2016-03-19T22:06:28.000Z

I don't know what else to do, clearing cache is not workign and I really need to be able to use wcjs-player in opened window. :S

Answer 14 · 2016-03-20T04:47:08.000Z

Uhm... I'm testing this in the simplest of ways, if you turn on the toolbar of nw.js, you'll see that it has 2 refresh buttons, one on the right, another on the left.

If you refresh the new window with the left refresh button, it just gives the same error again, but if you refresh it with the right button, the player starts in the new window.

A programatic refresh (location.reload()) results in reproducing the action of the left refresh button. I think the right refresh button is a node.js level refresh, I wonder how we can reproduce that action..

Answer 15 · 2016-03-20T05:33:18.000Z

Ok, I hacked it. :)

Proof of Concept

Example Usage:

var new_win = require('nw.gui').Window.open('index.html');
new_win.on('document-start', function() {
    new_win.reload(3);
});

Looking through other tickets related to this error, it seems that only NW.js has this issue. There have been users that switched to Electron to fix it.

Answer 16 · 2016-03-21T12:36:04.000Z

Now the player loads, but the global variable is brand new, not sharing the one it had with parent window. All the functions the parent window *loaded * on global variable are gone.

( from your proof of concept)

** index.html **

var wjs = require("wcjs-player");
var player = new wjs("#player").addPlayer({ autoplay: true });
global.HI = "yes global is here";
player.addPlaylist("http://archive.org/download/CartoonClassics/Krazy_Kat_-_Keeping_Up_With_Krazy.mp4");

new_window.html

console.log(global.HI);
var new_win = require('nw.gui').Window.open('index.html');
new_win.on('document-start', function() {
    new_win.reload(3);
});

The new_window.html output is undefined.

Answer 17 · 2016-03-21T12:51:08.000Z

Try using localStorage or process['env'] as a a replacement for whatever your using global for.

Answer 18 · 2016-03-21T12:53:55.000Z

Or if you have more complex needs you could connect the two windows with websockets to pass data between them.

Answer 19 · 2016-03-21T19:17:47.000Z

Appreciate all your help! I'll be trying Electron, if it doesn't work well I'll stick with NW.js and use localStorage. Thanks.

Answer 20 · 2016-09-20T10:25:51.000Z

Hi! I have the same problem and it's also connected to this issue
#34
That's all because of separate Javascript contexts introduced after NW v13 . By default nw app runs in the 'separate context mode' where we have Node context and Browser context and all required modules (like wcjs-player) are running in Node context which has it's own window object. That's why there are no styles and no DOM elements we're trying to get from wcjs-player module.

@Chayemor for me worked mixed context mode (in package.json add chromium-args: "--mixed-context") so any modules required with require() are running in the same context.

@jaruba since I have some other problems in mixed-mode and prefer not to use it if possible, I'd like to ask whether it will be possible to somehow wrap wcjs-player module and pass a proper window object (for example when we create a new player)?

Thanks

Answer 21 · 2016-09-20T11:27:54.000Z

@KiresMA I remember attempting that with a different user at one point (to pass the window object on player creation) and it didn't work for him for some reason..

We can try it again though, if you'd like.. it's not too complicated as you may imagine.. just replace all instances of window. with myWindow., then do:

var myWindow = window
var eventHooked = false

at the top of the script file, then delete these lines https://github.com/jaruba/wcjs-player/blob/master/index.js#L48-L69

and on this line https://github.com/jaruba/wcjs-player/blob/master/index.js#L222

add something like:

if (wcpSettings && wcpSettings.window) myWindow = wcpSettings.window

// deinitializate when page changed
if (!eventHooked) {
    eventHooked = true
    myWindow.addEventListener('beforeunload', function(e) {
        // stop all players
        for (var wjsIndex in players) if (players.hasOwnProperty(wjsIndex) && players[wjsIndex].vlc) players[wjsIndex].vlc.stop();

        // clear wcjs-player from require cache when page changes
        var clearModules = [
            "wcjs-player",
            "jquery" // https://github.com/jaruba/wcjs-player/issues/38
        ];
        for (var i in clearModules) {
            if (global.require.cache) {
                for (module in global.require.cache) {
                    if (global.require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete global.require.cache[module];
                }
            } else if (require.cache) {
                for (module in require.cache) {
                    if (require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete require.cache[module];
                }
            }
        }
    });
}

that should be everything.. then you can just do:

var player = new wjs("#player").addPlayer({
  window: window,
  autoplay: true,
  wcjs: require('wcjs-prebuilt')
  // OR
  // wcjs: require('webchimera.js')
  // OR
  // wcjs: require([path-to-Webchimera.js.node-file])
});

If you test this and it works for you, I'll gladly accept a Pull Request for it, as it's a very common issue with NW.js

Answer 22 · 2016-09-20T13:31:27.000Z

That different user probably was me :)
I tried it one more time and window object is used before addPlayer function.
Here https://github.com/jaruba/wcjs-player/blob/master/index.js#L41-L45
and here https://github.com/jaruba/wcjs-player/blob/master/index.js#L81
So when we create a new instance of wjs we still have non proper 'window'

that's why it didn't work for me last time and now either.

By the way, thank you for your quick answers! You are really quick in responses! :)

Answer 23 · 2016-09-20T14:20:36.000Z

https://github.com/jaruba/wcjs-player/blob/master/index.js#L41-L45 - This can also be moved in the if (!eventHooked) { ... } case.. but the other one.. uhmm.. that's tricky..

I think we thought about this the wrong way in the first place.. because the way I made wcjs-player was to replicate the developer experience of jQuery. So it makes sense that we should use jQuery like logic here too.. As with jQuery, window should be passed to wjs(), not .addPlayer(), as that's the main hook to the window and primarily used to attach to document elements..

So instead of doing:

wjs.prototype.addPlayer = function(wcpSettings) {
    if (wcpSettings && wcpSettings.window) myWindow = wcpSettings.window
    ...

What we should actually do is:

function wjs(context, newWindow) {
    if (newWindow) myWindow = newWindow
    ...

Then use it like:

var player = new wjs("#player", window).addPlayer({
  autoplay: true,
  wcjs: require('wcjs-prebuilt')
  // OR
  // wcjs: require('webchimera.js')
  // OR
  // wcjs: require([path-to-Webchimera.js.node-file])
});

And it should work! :)

Answer 24 · 2016-09-20T14:30:42.000Z

@KiresMA Good catch on https://github.com/jaruba/wcjs-player/blob/master/index.js#L81 , it all makes sense now..

I think this could even be taken one step further and we could localize each window to individual players, thus fixing issues such as multiple open windows with different players, etc. once and for all

Answer 25 · 2016-09-20T15:08:35.000Z

Here's my final train of thought..

https://github.com/jaruba/wcjs-player/blob/master/index.js#L38-L69

Should be changed to:

function killPlayers() {
    // stop all players
    for (var wjsIndex in players) if (players.hasOwnProperty(wjsIndex) && players[wjsIndex].vlc) players[wjsIndex].vlc.stop();

    // clear wcjs-player from require cache when page changes
    var clearModules = [
        "wcjs-player",
        "jquery" // https://github.com/jaruba/wcjs-player/issues/38
    ];
    for (var i in clearModules) {
        if (global.require.cache) {
            for (module in global.require.cache) {
                if (global.require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete global.require.cache[module];
            }
        } else if (require.cache) {
            for (module in require.cache) {
                if (require.cache.hasOwnProperty(module) && module.indexOf(clearModules[i]) > -1) delete require.cache[module];
            }
        }
    }
}

function attachKiller(window) {
    // deinitializate when page changed
    window.removeEventListener('beforeunload', killPlayers)
    window.addEventListener('beforeunload', killPlayers)

    // inject css
    if (!$("link[href='"+relbase+"/css/general.css']", window.document).length) {
        $('<link href="'+relbase+'/css/general.css" rel="stylesheet">', window.document).appendTo("head");
        window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar','width: 44px !important;');
        window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar-track','background-color: #696969 !important; border-right: 13px solid rgba(0, 0, 0, 0); border-left: 21px solid rgba(0, 0, 0, 0); background-clip: padding-box; -webkit-box-shadow: none !important;');
        window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar-thumb','background-color: #e5e5e5; border-right: 13px solid rgba(0, 0, 0, 0); border-left: 21px solid rgba(0, 0, 0, 0); background-clip: padding-box; -webkit-box-shadow: none !important;');
        window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar-thumb:hover','background-color: #e5e5e5 !important; border-right: 13px solid rgba(0, 0, 0, 0); border-left: 21px solid rgba(0, 0, 0, 0); background-clip: padding-box; -webkit-box-shadow: none !important;');
        window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar-thumb:active','background-color: #e5e5e5 !important; border-right: 13px solid rgba(0, 0, 0, 0); border-left: 21px solid rgba(0, 0, 0, 0); background-clip: padding-box; -webkit-box-shadow: none !important;');
    }
}

Then we'd go here:
https://github.com/jaruba/wcjs-player/blob/master/index.js#L71

And change it to something like:

function wjs(context, newWindow) {
    this.window = newWindow || window
    attachKiller.call(this)
    ...

Then replace all occurrences of window. with this.window. (even in the new attachKiller() function, but make sure to keep this.window = newWindow || window intact when replacing)

I hope I made it all clear enough.. And if it works, we'd have localized windows for each player :)

Same usage would apply:

var player = new wjs("#player", window).addPlayer({
  autoplay: true,
  wcjs: require('wcjs-prebuilt')
  // OR
  // wcjs: require('webchimera.js')
  // OR
  // wcjs: require([path-to-Webchimera.js.node-file])
});

Answer 26 · 2016-09-20T16:14:11.000Z

I tested the last code I recommended with NW.js 0.17.1 32bit, and something very strange is happening..

I still get:

Uncaught TypeError: Cannot read property 'addRule' of undefined

from:

this.window.document.styleSheets[0].addRule('.wcp-menu-items::-webkit-scrollbar','width: 44px !important;');

but if i do:

this.window.console.log(this.window.document.styleSheets[0].addRule)

right above it, it logs:

function addRule() { [native code] }

correctly.. same goes for everything else in the module, it's like although I'm sending everything to the proper window now, it still errors out for no reason.. so something is definitely wonky in NW.js these days.. i have no idea how i could fix this silliness..

Answer 27 · 2016-09-21T06:58:10.000Z

Well, for me it's the same situation :(

Answer 28 · 2016-09-21T15:51:31.000Z

i think this should be discussed with the NW.js guys directly, but they rarely if ever answer any of their issues (or at least that was the case in the past)

i switched to using Electron instead of NW.js in my projects just because of the fact that the Electron guys always answer their issues, and actually want to help fix them too