Skip to content

Scope ModelContext to Document, and handle detached documents#177

Open
domfarolino wants to merge 1 commit into
mainfrom
fix-modelcontext-lifetime
Open

Scope ModelContext to Document, and handle detached documents#177
domfarolino wants to merge 1 commit into
mainfrom
fix-modelcontext-lifetime

Conversation

@domfarolino
Copy link
Copy Markdown
Collaborator

@domfarolino domfarolino commented May 15, 2026

This PR does two things:

  • Scopes the ModelContext object to Document instead of Navigator, and explains why.
  • Makes the modelContext attribute return null when the document is detached (by checking when the Window's browsing context is null). This matches the behavior of the contentWindow and contentDocument getters—return nothing, even if those objects are allocated and kept alive by other references, in the detached document case.

Note that this PR does not handle the "document not fully active" case, which covers detached documents and documents in the bf-cache. Non-fully active documents probably shouldn't impact the modelContext getter—it seems weird for getters like that to return different values based on whether the document is in the bf-cache—but rather need special behavior in the registerTool() method, and the coming getTools() and executeTool() methods.

This PR mostly addresses #173, with the exception of considering moving the API to self, from navigator. As much as I want to do this, I'm sympathetic to w3ctag/design-principles#426 (comment), so for now I'm leaning towards not moving it. So I'll say that this PR closes #173.


Preview | Diff

@domfarolino domfarolino requested a review from bwalderman May 15, 2026 23:33
Comment thread index.bs
{{ModelContext}} object.

Upon creation of the {{Document}} object, its [=Document/associated ModelContext|associated
<code>ModelContext</code>=] must be set to a [=new=] {{ModelContext}} object created in the
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hey @tabatkins, I could not for the life of me figure out how to add {{ModelContext}} or <code>ModelContext</code> to the actual linking text, so I had to settle on this super verbose [=foo|<code>foo</code>=] which is really redundant and I can't figure out if there's a way around it, so I can tighten these up. It's not blocking, so if there's no solution, that's fine.

@domfarolino domfarolino changed the title Scope and null-out for detached documents Tie ModelContext lifetime to Document and handle detached documents May 15, 2026
@domfarolino domfarolino changed the title Tie ModelContext lifetime to Document and handle detached documents Scope ModelContext to Document, and handle detached documents May 15, 2026
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.

Make tools Document-scoped instead of Window-scoped

1 participant