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

Should we assign #document-state-origin on new child navigable creation? #9460

Closed
domfarolino opened this issue Jun 27, 2023 · 3 comments · Fixed by #9494
Closed

Should we assign #document-state-origin on new child navigable creation? #9460

domfarolino opened this issue Jun 27, 2023 · 3 comments · Fixed by #9494

Comments

@domfarolino
Copy link
Member

Upon creation of an iframe and its initial about:blank document, I think we never end up assigning the new document state's origin or initiator origin.

  1. We insert the iframe.
  2. This enters https://html.spec.whatwg.org/#creating-a-new-browsing-context where we compute the origin and assign it to the Document.
  3. We pop back to the later steps in https://html.spec.whatwg.org/#create-a-new-child-navigable where we create the document state for the new document, only populating two of its members: document, navigable target name
  4. We pop back to the insertion steps and enter the https://html.spec.whatwg.org/#process-the-iframe-attributes, which silently returns in step 2.3, since this is the initial about:blank Document.

I think this means that if we navigate forward to FOO and then traverse back to the initial about:blank Document, and it has been destroyed, repopulating it will reproduce a document with the about:blank URL (because its SHE's URL was set up right in https://html.spec.whatwg.org/#initialize-the-navigable), but with an opaque origin. I think this happens because the target SHE's document's state's initiator was not set, so we pass null in as sourceOrigin in https://html.spec.whatwg.org/#populating-a-session-history-entry:determining-the-origin-2, so we just return about:blank's origin which is opaque.

So not only do we never set initiator origin (because we never enter #navigate, the only place initiator origin is written to: https://html.spec.whatwg.org/#beginning-navigation:document-state-initiator-origin) but I think we never end up setting document state's "origin" either, because even though the new child navigable creation steps run apply the history step (IIUC), they never seem to enter attempt to populate, which is the only place "origin" is assigned.

It would be good if someone like @domenic could sanity-check my finding here to see if I've actually identified an issue here or if I just missed something.

@domenic
Copy link
Member

domenic commented Jun 28, 2023

I traced through your logic and I think you're correct. It would be good to set both initiator origin and origin, most likely!

which is the only place "origin" is assigned.

It's also populated in navigate, when heading to an about:blank/about:srcdoc document.

(In general, it's easy to create problems like this where we forgot the initial about:blank, precisely because it goes through none of the main algorithms...)

@domenic
Copy link
Member

domenic commented Jul 12, 2023

I'm actually pretty confused now. Looking at all the places that create a "new document state":

  • navigate step 19 sets initiator origin, and sometimes sets origin.
  • javascript: URL sets origin, but the link there to "origin" actually links to the definition of "initiator origin"!?
  • create navigation params by fetching only sets origin.
  • create a new top-level traversable / create a new child navigable, as you note, don't set either.

Should we fix all of these at once? Maybe we could make origin / initiator origin non-nullable at the same time?

@domenic
Copy link
Member

domenic commented Jul 12, 2023

  • navigate step 19 sets initiator origin, and sometimes sets origin.

This is probably OK. Origin will get set later, at document creation, in "attempt to populate the history entry's document".

domenic pushed a commit that referenced this issue Jul 14, 2023
This fixes #9460 by supplying an initiator origin (or explicitly null) everywhere we create a new document state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants