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

Make the global object usable interchangeably with the window object #833

Closed
jdm opened this issue Aug 31, 2013 · 9 comments
Closed

Make the global object usable interchangeably with the window object #833

jdm opened this issue Aug 31, 2013 · 9 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Aug 31, 2013

I'm not sure how to do this yet. A resolve hook might work, but that's apparently frowned upon quite severely in modern SpiderMonkey.

@jdm
Copy link
Member Author

@jdm jdm commented Aug 31, 2013

Needed for #841 (it uses a bare setTimeout, at the very least).

@BrendanEich
Copy link

@BrendanEich BrendanEich commented Aug 31, 2013

What problem is this bug fixing? The global object must be the window object, by definition. It has a self-referencing property named 'window' (among others). Is this about resolving 'window', or does the summary imply there are two objects with distinct identities?

/be

@jdm
Copy link
Member Author

@jdm jdm commented Aug 31, 2013

Right now there are two very distinct objects (the global object is pretty much bare). This bug is about rectifying that (or at least papering it over for now).

@BrendanEich
Copy link

@BrendanEich BrendanEich commented Aug 31, 2013

Yikes. They can't differ. That is,

assert(this === window)

must be true in JS in the browser.

Why do they differ? Sorry for not knowing.

/be

@jdm
Copy link
Member Author

@jdm jdm commented Aug 31, 2013

They differ because we just throw together a random bare global object and only later define our various bindings, at which point we set a window property that is actually the real Window object. Obviously this is bad and wrong; it's just something that needs fixing.

@BrendanEich
Copy link

@BrendanEich BrendanEich commented Aug 31, 2013

Sure, I got there are two objects -- was really asking why?

The 'window' binding can be predefined, as in Gecko in nsGlobalWindow::SetNewDocument.

/be

@bzbarsky
Copy link
Contributor

@bzbarsky bzbarsky commented Sep 1, 2013

Brendan, the global is not the same JSObject* as the window in Gecko either, as you well know. === outerizes and lies; you can actually detect the difference between the two objects by comparing barewords to window.foo in a navigated-away-from page.

But the key is that setTimeout() with null thisobj should find its correct global. Which is not the same as .window on that global, note, in the navigated-away-from case. Right now in Gecko, we use JS_THIS, which computes us to the window, but I consider that a bug in Gecko that I plan to fix.

jdm: what you need to do is to tell the JS engine that the global's outer is the window.... There are class hooks for this, iirc.

@BrendanEich
Copy link

@BrendanEich BrendanEich commented Sep 1, 2013

@bzbarsky: thanks, sorry about that -- vacation brain thought it could type for some reason ;-). The assertion has to hold but both this and window must be "outerized" to WindowProxy, while the "cannot be referenced from user-code" global remains Window.

I was still curious if there was a Rust or Servo issue motivating the global being other than Window. It sounds like temporary staging work.

/be

@jdm
Copy link
Member Author

@jdm jdm commented Sep 1, 2013

It was just a situation of me building on the JS stuff that brson put together a long time ago and never getting round to doing it correctly

jdm added a commit to jdm/servo that referenced this issue Sep 6, 2013
jdm added a commit to jdm/servo that referenced this issue Sep 6, 2013
@jdm jdm closed this in da599c6 Sep 10, 2013
ChrisParis pushed a commit to ChrisParis/servo that referenced this issue Sep 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.