Ckeditor5: The isWindow helper fails when running the editor in electron

[li-rongcheng]

Is this a bug report or feature request? (choose one)

🐞 Bug report

💻 Version of CKEditor

"@ckeditor/ckeditor5-build-classic": "^1.0.0-alpha.2"

📋 Steps to reproduce

1. Embed ckeditor5 in electron (along with angular5)
2. Type enters in the editing area

✅ Expected result

There should be no error report in the developer tool's console window.

❎ Actual result

Every time when enter is hit, an error "cannot read property 'defaultView' of undefined" is reported in the console:

📃 Other details that might be useful

When run the app in browser, no such errors are seen. So it only happens in electron.

I have uploaded the source to https://github.com/li-rongcheng/electron-angular5-ckeditor5 .
To reproduce the issue, do:

ng build
npm run elec

then create an ckeditor5 instance and hit 'enter' in the editing area.

I think this might be a ckeditor5's bug because when I use ckeditor4 or other online editors like quill, tinymce or trix, everything were correct, i.e. it only happens in ckeditor5.

[Reinmar]

cc @oleq

[oleq]

I tried to run the code but all I got is

[li-rongcheng]

@oleq It's weird, can you try it in another driver and path? BTW, I tried again also in my users path, didn't get your problem, here is how I do it:

[oleq]

I managed to run the code and I found the root of the problem. The problem is in the isWindow helper where we check if passed argument is an instance of the window.

return Object.prototype.toString.apply( obj ) == '[object Window]';

but in electron, it is global instead of window so the string is '[object global]'.

This false negative changes the flow of the application in Rect#excludeScrollbarsAndBorders() and we end up with an error. BTW, we use isWindow() in the EmitterMixin too, so probably this is not the only error we would see.

[Reinmar]

Would it make sense to call it isGlobal() and check for both things inside – window and Electron's global? Does Electron's global object behave the same way (in terms of events, etc.)?

[Reinmar]

BTW, there's one more thing. When working on https://github.com/ckeditor/ckeditor5-utils/blob/master/src/translation-service.js I realised that we need a safe way of accessing the global object. Also, it's a question whether the global object is part of the DOM or not, because the globals.window property is in src/dom/globals now and perhaps the global object should be somewhere in src/globals.

[printjs]

@Reinmar @li-rongcheng @oleq How to solve this problem？

I get same problem under electron

[printjs]

The error message is always thrown when the user enters.the sitution really annoying

[Reinmar]

It's more of a question to you, guys. We don't know Electron – we'd have to research how things work inside it. If you can find the right way to check if something is the equivalent of the browser's window and how to access this element (if we need it – e.g. to listen to scroll event), then please make PRs.

[ma2ciek]

I've checked and the window object is still accessible and it's a reference to the global object.

After changing isWindow function to:

export default function isWindow( obj ) {
    const stringifiedObject = Object.prototype.toString.apply( obj );

    return stringifiedObject == '[object Window]' || stringifiedObject == '[object global]';
}

(ofc it would be better to rename it to isGlobal).

everything works fine for the https://github.com/ckeditor/ckeditor5-build-classic.

[Reinmar]

Cool. Could you make a PR?

I'm not sure about renaming it, though. And I think we may need to extend this check with one more thing.

In Node.js there's global too and it stringifies to [object global]. However, it doesn't have the addEventListener() method. In other words, it's not a DOM's global. And what we need here is to check if that object is a DOM's global object.

So, first of all, it's fine that this function is inside utils/dom/. That's where we need it.

Secondly, I think it's ok that it's called isWindow() because we're checking for that object.

Thirdly, we may (but don't have to so far) add an additional check that there's an addEventListener method on it. That's to exclude non-DOM globals.

[ma2ciek]

I made a PR.

In Node.js there's global too and it stringifies to [object global]. However, it doesn't have the addEventListener() method. In other words, it's not a DOM's global. And what we need here is to check if that object is a DOM's global object.

So, first of all, it's fine that this function is inside utils/dom/. That's where we need it.

Secondly, I think it's ok that it's called isWindow() because we're checking for that object.

That makes sense to me as we need window object, not global.

Thirdly, we may (but don't have to so far) add an additional check that there's an addEventListener method on it. That's to exclude non-DOM globals.

I'd wait with it if it's not needed for now.