Problem with gui.Window.get() #303

Closed
owenc4a4 opened this Issue Dec 31, 2012 · 11 comments

Projects

None yet

3 participants

@owenc4a4
Contributor

Run code below.

var win = window.open('text.html');
gui.Window.get(win);

Then there will be error, if test.html has nodejs code.
This wouldn't appear in v0.3.6.

@zhchbin
Contributor
zhchbin commented Jan 5, 2013

This bug is caused by the commit: 8f99f3f

@rogerwang
Member

why the error is expected? or why is this a bug?

@zhchbin
Contributor
zhchbin commented Jan 5, 2013
var newWindow = window.open('text.html');
var win = gui.Window.get(newWindow );

With these code, the win should be a Window Object. Then the user can use the win to operate the new opened window in the index.html. However, the gui.Window.get(newWindow) need node, and (force_on || is_file_protocol) stop the new window to integrate node.

Here is the error message from the console when I call the previous code:
QQ 20130105215716

I just have a try that remove (force_on || is_file_protocol). And everything work.

@rogerwang
Member

Thanks. But are you sure the offending commit is 8f99f3f ?

@zhchbin
Contributor
zhchbin commented Jan 5, 2013

I think I can make sure, I remove the (force_on || is_file_protocol), (which will be false when using window.open('test.html')) then it will work. BTW v0.3.6 didn't have this bug.

@rogerwang
Member

this commit 1500e6d touches the logic for is_file_protocol too ...

@zhchbin
Contributor
zhchbin commented Jan 5, 2013

Yeah, you are right then. I miss this, sorry... I think I should said it is the (force_on || is_file_protocol)

@rogerwang
Member

thanks for pinpointing the error :) I'll fix this in 0.3.7

@rogerwang
Member

@owenc4a4 please add it to test cases.

@owenc4a4 owenc4a4 was assigned Jan 5, 2013
@rogerwang rogerwang added a commit that closed this issue Jan 5, 2013
@rogerwang rogerwang install Node for window.open() Windows
In previous v8 code, in the middle of loading, KURL returns about:blank
for invalid URL. So we simulate the same behaviour here.

Fix #303
a163a27
@rogerwang rogerwang closed this in a163a27 Jan 5, 2013
@rogerwang
Member

reopen to track the test case.

@rogerwang rogerwang reopened this Jan 5, 2013
@owenc4a4
Contributor

test in app_tests/calls_across_window.

@owenc4a4 owenc4a4 closed this Jan 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment