document state's history policy container does not seem to be correctly updated

[antosart]

I'm not sure I fully understand the spec yet after the navigation part was rewritten, but I think there might be a few problems with document state's history policy container.

• I don't see where document state's history policy container is set to something which is not "client". Going through the references, I only see it being set here and here. In both cases it's set to a clone of another document state's history policy container. So how can that policy container ever become different than "client"?

• Originally, history policy container was only set if the url required storing the policy container in history. This (maybe fragile) logic was needed to decide whether to restore the policy container from history in determine navigation params policy container. Now I don't follow how that should work anymore, and how this assertion can be satisfied.

• document state's history policy container is either "client" or a policy container. However, this value is fed into determining navigation params policy container which expects a policy container or null.

I hope this makes sense - maybe I am just missing something.

[antosart]

cc @domenic

[domenic]

@domfarolino you were most familiar with this part of the rewrite. Would you be able to take this on? It seems somewhat bad and is semi-blocking some other work so if you're not available soon let me know and I'll try to figure it out.

[domfarolino]

I'll assign myself for now, and try my best to look at it this week and see what I think.

[domfarolino]

That is a pretty glaring bug, thanks for filing this. #she-policy-container is the pre-rewrite equivalent to today's #document-state-history-policy-container. It used to be a policy-container-or-null (never "client") and I think we need to do the same for document state's history policy container. I'm guessing the "client" value got slipped in by accident during the rewrite.

I guess that's step 1, but then we actually need to set document state's history policy container to the one in the populated navigation params, provided the policy container needs to be stored in history. Pre-rewrite, this was handled by #update-the-session-history-with-the-new-page (#navigating-across-documents:requires-storing-the-policy-container-in-history-2 specifically, and one more identical location), but now it isn't done at all.

Somewhere after navigation params is fully populated, we need to conditionally set the entry's document state's history policy container to the one in navigation params. I'll start exploring exactly where this should be in the new setup.