Calling InitializeHostDefinedRealm in document.open is probably unnecessary

[rniwa]

According to my testing neither WebKit nor Blink does the equivalent of calling InitializeHostDefinedRealm in step 16 of document.open.

WebKit's document.open and its [JavaScript binding code](https://trac.webkit.org/browser/trunk/Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp?rev= 204629#L153), for example, just sets up the parser state, clears event listeners, & resets some security context, but goes ahead and uses the same global object.

In fact, it's quite possible that a whole bunch of mobile content are dependent on this behavior given the combined market share WebKit and Blink has had in the last decade. In fact, it's easier for author to depend on the fact the global object persists over document.open than that they change. e.g. shared global states, instanceof checks, etc...

[rniwa]

@annevk @domenic @bzbarsky

[bzbarsky]

WebKit/Blink also don't implement anything like the session history behavior Firefox has long had for document.open; instead they do something much less user-friendly....

It used to be the case that document.open could change origins (though not effective script origins). At that point, reusing the global was an obvious security non-starter from my point of view. I think at this point neither can change on open() (this would need to be verified). If so, we can at least think about this. The session history interactions in the more user-friendly model still end up pretty weird, of course, but I've accepted that WebKit/Blink don't care about the user experience around document.open and that nothing I say will get them to change that....

That said, keeping the same script state but clearing event listeners is pretty weird in its own right: it can cause the still-there scripts to no longer work correctly because the event listeners they depend on are missing.... So there can be web compat fallout from changing to the WebKit/Blink behavior, just as vice versa. :(

[annevk]

@bzbarsky could you explain what is not user friendly? Note that per WebKit's binding code it does seem like the origin is changed (they call it "security context").

[bzbarsky]

@bzbarsky could you explain what is not user friendly?

Because you can't do back/forward across the document.open call to get to the page before open got called.

Just try this testcase: data:text/html,<script>var x = 0;</script><iframe></iframe><input type="button" value="Click me" onclick="var doc = frames[0].document; doc.open(); doc.write(++x); doc.close()"> and observe the behavior difference between Firefox and Chrome in terms of session history.

Note that per WebKit's binding code it does seem like the origin is changed (they call it "security context").

Changing the origin (as opposed to effective script origin) of a Window object doesn't seem at all OK to me. Nothing in the platform does that right now, to my knowledge.

[annevk]

We no longer have an effective script origin concept. I'm not entirely sure what Document* activeDocument = asJSDOMWindow(state.lexicalGlobalObject())->wrapped().document(); does, but given that it seems to require document access, it's likely not that severe.

[bzbarsky]

We no longer have an effective script origin concept.

Well, sure. I meant changing origin as opposed to changing "domain".

it's likely not that severe

I'm not sure what you mean by "severe". My point was that currently the origin "pointer" of a document is immutable in the specs, and I see no reason to change that. Having it be immutable allows certain optimizations that would not be possible otherwise, for example.

[foolip]

Looks scary. CC @mikewest and @jeisinger who may understand this code in Blink, or perhaps such a person doesn't exist.

[jeisinger]

hum, in #583 we changed document.open() to abort if the origins of the caller and the document don't match, so I don't think we can change the origin here anymore.

[jeisinger]

note that blink's document.open implementation is also pretty far away from what the spec describes :-/

[annevk]

@jeisinger could you describe what you are doing now or maybe point towards the code? Is it different from what WebKit does? I think changing the specification here to not allocate a new global object would be a great simplification to the model we are describing today.

[jeisinger]

our code is here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Document.cpp?rcl=0&l=2499

The main difference appears to be that we have the same-origin check which WK does not.

[annevk]

Okay, so why do you still set the origin? Is that even observable?

[bzbarsky]

I think not creating a new global is really weird, because it involves clearing some, but not all, state on the old global. That's really confusing to authors, because some of their things stick around, some do not, and they have no idea which is which.

[annevk]

Either way what happens is really weird. Best to not use window.open() on yourself (or at all).

Setting the origin to that of the entry settings object is observable as setting document.domain afterwards would affect both sides, rather than one.

I think it's worth making this change so that Document objects are almost associated with one global object. And per https://www.w3.org/Bugs/Public/show_bug.cgi?id=20567#c75 it seems Edge would support this too.

[bzbarsky]

Setting the origin to that of the entry settings object is observable

Yes, we don't want to do that at all.

I think it's worth making this change

I don't think it is and would need some very careful analysis of any new proposed behavior, its interactions with navigation, and its possible web compat impacts to be comfortable making any changes here.

[bzbarsky]

I should note that you never did address my concerns about user experience and back/forward navigation....

[annevk]

Basically, it does not seem worth caring about document.open() doing the "right" thing. And if we can drastically simplify the overall model by adopting what the majority of browsers do, we should.

[bzbarsky]

Basically, it does not seem worth caring about document.open() doing the "right" thing.

I think we simply disagree on that.

And if we can drastically simplify the overall model by adopting what the majority of browsers do, we should.

If it's actually a simplification and if the "majority of browsers" are actually all doing the same thing and if the spec properly figures out that thing and if that thing doesn't depend on other non-spec-matching behaviors in those browsers. I've been burned so many times recently by HTML spec changes that failed to actually check some of those things, that I'm very wary of them, sorry. :(

[annevk]

That's fair and I certainly wouldn't expect Gecko to make a move here anytime soon. We basically need to do a lot more to remove technical debt around the lifecycle of documents, global objects, and browsing contexts. And make sure all of that also ends up getting tested properly.

In order to get there though we will need to make some decisions along the way, this being one of them.

[bzbarsky]

OK, but I have yet to see a clear description of what the "other model" even is. That seems like a minimal bar to even having a meaningful conversation here....

[nox]

@rniwa Does WebKit even prompt to unload, unload and then abort the Document before creating the new parser etc? Does it cancel ongoing loads? Asking because I'm implementing this in Servo.

[nox]

AFAICT, WebKit also doesn't do anything with regard to timers, and it just let them run until their completion.

[jeisinger]

@annevk asked me to spell out the document.open steps of Blink. Here we go:

1. if the document was imported (via HTML imports), abort with an InvalidStateError
2. like step 1) in the spec
3. like step 2) in the spec
4. like step 4) in the spec
5. set document's origin to the entered settings object's responsible document (or whatever the right spec term is) origin
6. if the document and the entered settings object's responsible document differ, set the document's URL to the entered... document'sURL with the fragment stripped. (almost step 23 of the spec)
7. set the document's cookie URL to the entered... document's cookie URL
8. if the document is active, run step 5 of the spec
9. if the document is active, and has an active parser that wasn't created by script, and has an insertion point, return document
10. "stop all loaders". I guess that's step 12) in the spec?
11. step 13 of the spec
12. clear any selection associated with document
13. step 15 of the spec
14. set document's compat mode to "no quirks"
15. step 25 of the spec
16. step 26 of the spec
17. if "load event progress" is not "load event in progress" and we're not dispatching a page dismissal event, set "load event progress" to "load event not run" (not sure what this is in spec terms)
18. mark the loader associated with document to have committed its first real load if it hasn't done so yet
19. notify our loading machinery that a load has started unless the document's parent is done loading or in the process of completing
20. cancel ongoing navigations for this document
21. return document

hope that helps :/

[wanderview]

FWIW, I was curious how the different document.open() behavior might impact service workers and clients API. I did some testing:

The test I ran was:

1. Open https://fetch-event-echo.glitch.me/
2. Open web console
3. Check value of navigator.serviceWorker.controller
4. Execute fetch('/') and note the client ID echo'd by the service worker.
5. Execute document.open();document.close();
6. Repeat steps 3 and 4 and compare.

I mainly checked firefox and chrome since that is what I have available on this machine:

Firefox 57:

• controlled by same service worker before and after
• same client ID before and after

Firefox 59:

• controlled by same service worker before and after
• different client ID after

Chrome 63 and 65:

• controlled by same service worker before and after
• same client ID before and after

The fact we inherit the controller is better interop than I expected.

The client ID should reflect if a new global is created or not. Using the same client ID in chrome makes sense since it does not create a new global. In FF57 we had a bug because we were treating the document as the Client (and document.open does not create a new document object). In FF59 we now treat the window global as the client, so we get a new ID after document.open().

[futurist]

For the session history problem, I think it should be managed by the developer when he/she wrote document.open, then history can be managed by history API like history.pushState, history.replaceState etc.

Since the box is opened by developer, it's their responsibility to manage that, thus more control of the history rather than managed by the browser.

[bzbarsky]

So after document.open(), whether you follow the Firefox/spec model or the currently-underspecified everyone-else model, window.customElements should change value.

@domenic do you still think that? It looks like the final changes to document.open do not in fact change the value of window.customElements...

//cc @TimothyGu

[bzbarsky]

And similar things for other thing like the actual custom element registry, the Location object, etc.

In general, searching for https://github.com/whatwg/html/issues/1698 in the wpt repo finds a number of hits that all look like they fail with the current spec...

[TimothyGu]

I believe when @domenic and I discussed this, we decided that the current spec is probably a better choice. I didn't think to search for this issue when making the actual WPT changes however, as an oversight.

[domenic]

Yes, sorry for not having a clear conclusion recorded here. We decided that in the new model, not resetting everything was simpler, and the current spec reflects that. We tried to update the tests we knew about, but I guess you found more. Will you be able to fix those tests as part of your Gecko work, or should we try to do so ourselves?

[bzbarsky]

I can fix the tests as part of the Gecko work, but I don't have a timeframe for it yet. It's a sort of spare-time project and running into unforeseen difficulties so far in terms of number of failing tests of various sorts.

[domenic]

No worries. Let us know if you end up putting it on indefinite hold, in which case we'll take over to ensure things settle into a consistent state before everyone forgets all the details again.