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

Begin working on creating a BrowsingContext #1909

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ForbesLindesay
Copy link
Contributor

This is a work in progress, not ready to merge, but I’d appreciate assistance and pointers as I’m getting started.

Which objects in jsdom are representing the Window vs. WindowProxy?

Does “get the [[Foo]] Internal property of X” translate to approximately:

idlUtils.implForWrapper(X)._foo

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which objects in jsdom are representing the Window vs. WindowProxy?

jsdom does not have WindowProxy at all right now. This is basically because WindowProxy is only relevant when you navigate, and even then only when you navigate a frame. It helps enforce that iframe.contentWindow stays the same object (a WindowProxy) even though the underlying global (a Window) completely changes.

I think it'll be important eventually, although for now I'd be OK with breaking that equality to get things off the ground, especially since (as noted in #1903) I am unsure about the performance implications. However, not having WindowProxy will probably make it very hard to pass a number of web platform tests related to navigation :-/.

Does “get the [[Foo]] Internal property of X” translate to approximately:

Yes, that sounds perfect.


// 2. Call the JavaScript InitializeHostDefinedRealm() abstract operation
const window = new Window();
// TODO: InitializeHostDefinedRealm({globalObject: window, globalThisValue: this.windowProxy, relmExecutionContext: new JavaScriptExecutionContext})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this TODO is very actionable.

Creating a new realm is basically the vm.createContext() business, which the Window constructor takes care of. I'd maybe revise this to

// 2. Call ...
// In the current implementation, the Window() constructor takes care of creating a new realm via the vm module

// https://html.spec.whatwg.org/#windows
class BrowsingContext {
// readonly windowProxy: WindowProxy
// readonly sessionHistory: SessionHistory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of these myself

document: idlUtils.implForWrapper(this._document),
url: idlUtils.implForWrapper(this._document)._URL,
stateObject: null
}];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So currently all the history-tracking is done by the History object, which hangs off of the window. I guess we need to disentangle them so that session history survives navigation. Then History will be an interface into a SessionHistory.


// create a new browsing context
constructor() {
this.windowProxy = new WindowProxy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't implement WindowProxy for now, we could just say this.windowProxy = window with a comment about how it's an interim solution, but the intent is that the public API presented by BrowsingContext can be reused later even when we implement a proper WindowProxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants