Skip to content
This repository was archived by the owner on Sep 7, 2018. It is now read-only.

WIP: Change Electron to use window.open#195

Closed
BenLambertNcl wants to merge 1 commit intosymphonyoss:masterfrom
BenLambertNcl:electron-window-change
Closed

WIP: Change Electron to use window.open#195
BenLambertNcl wants to merge 1 commit intosymphonyoss:masterfrom
BenLambertNcl:electron-window-change

Conversation

@BenLambertNcl
Copy link
Collaborator

This is a work in progress for changing the Electron API to use window.open. This is so child windows are created in the same process as their parent, therefore allowing access to each others JavaScript context.

WIP because features string doesn't work yet, meaning tests also fail.

@ColinEberhardt
Copy link
Collaborator

@BenLambertNcl I guess this means that it is looking like a viable option to have ContainerJS adopt a more OpenFin-like model instead?

@ColinEberhardt
Copy link
Collaborator

One thing to be wary of is that the electron behaviour you get from window.open is different when the sandbox is enabled:

https://github.com/electron/electron/blob/master/docs/api/sandbox-option.md

I must admit, I hadn't turned it on in my own investigations!

@BenLambertNcl
Copy link
Collaborator Author

BenLambertNcl commented Jun 12, 2017

@ColinEberhardt Yeah we can use the OpenFin process model with this change. I think its much nicer this way, as we give the user a choice, rather than forcing them into 1 specific process model. As for the link I hadn't seen that! It seemed to have all of the methods it mentioned in the documentation on the object when I tested it. Will double check though.

@BenLambertNcl
Copy link
Collaborator Author

BenLambertNcl commented Jun 13, 2017

@ColinEberhardt From further research, it seems that we might need to give up some functionality using window.open due to the lack of window options that are supported when creating a window. The list that I have made of options that do not work with window.open are:

  • maxWidth
  • maxHeight
  • minWidth
  • minHeight
  • alwaysOnTop
  • backgroundColor
  • center
  • frame
  • maximizable
  • minimizable
  • resizable
  • show
  • skipTaskbar
  • transparent

Some of these can be set after the window has opened (such as resizable, minimizable etc.) but some like show and frame, only work when passed into the window constructor. It seems like some of these should work though, with references to show here as well as references to max/min width and others here, but none do, even in the electron api demos.

Not sure if it is worth continuing with this? Seems odd that these options are used in the source code and they do not work, could be a bug in Electron?

@ColinEberhardt
Copy link
Collaborator

I must admit, I'm not too surprised - the MDN page for window.open does discourage its usage.

It might be worth parking this.

My preference is that you try to create a consistent process model between containers, but for the initial implementation I'm not too fussed about what that actually is.

Take the easiest path!

@BenLambertNcl
Copy link
Collaborator Author

Ok I will stick to the "Each window as a separate process" like we were doing initially then. Ill close this PR for now, but we can always reopen it if we see value in using this later.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants