Navigation: Clarification on ever populated flag of DocumentState during apply the history step

[ADKaster]

What is the issue with the HTML Standard?

I'm a bit confused by the assertion in step 12.5 for replace.

In Ladybird on my navigation update branch, I hit that assertion when navigating from the initial about:blank state to about:blank. Our headless test runner navigates to about:blank between each test run to get a clean slate.

Are we possibly missing some implementation?

From what I understand, navigation must be a replace when the document is the initial about:blank, so perhaps the assertion or other spec steps don't take into account what happens when you navigate from that to about:blank? Or perhaps we're supposed to discard such a navigation as a no-op?

[ADKaster]

I'm also confused why the check that the target entry's document is not ever populated in general..

We set ever_populated to true in populate_session_history_entry_document step 6.7.1 when the document is non-null, and then call into the completion steps. Which in the case of navigate a navigable step 20.9.1 will be to finalize a cross document navigation. Which in turn applies the push/replace history step, and enqueues a global task to the navigation and traversal task source of the active window to do step 12.5 of apply the history step.

So... In all cases(?) we will populate the session history entry document before ever checking the navigation type, and we will always have populated the target entry?

Unless I'm completely misreading :)

[domenic]

Yes, this looks like a bug to me, maybe based on misunderstanding when the "ever populated" value gets set. /cc @noamr to confirm, since he added that assert.

I think the intent was something like, "assert that we're going to a new entry which we haven't visited previously". But, as you note, "ever populated" gets set before "apply the history step" is called, even for "new" entries (i.e. the push/replace cases).

I think probably the best fix here is to remove the asserts related to ever populated (which would mean no asserts for "traverse").

[noamr]

Good catch, SGTM