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

Steps for attempt to populate the history entry's document seem to be missing a level of nesting #9767

Closed
ADKaster opened this issue Sep 21, 2023 · 1 comment · Fixed by #9902
Assignees

Comments

@ADKaster
Copy link
Contributor

in https://html.spec.whatwg.org/multipage/browsing-the-web.html#attempt-to-populate-the-history-entry's-document, step 6 starts a global task on the navigation and traversal task source

Step 3 of the global task steps says:

If navigationParams is a non-fetch scheme navigation params, then set entry's document state's document to the result of running attempt to create a non-fetch scheme document given navigationParams.

So at this point, we've created a new document for the navigation history entry's document state using hand wavy implementation defined methods for non-fetch scheme urls.

Steps 4, 5, 6, and 7 then set a "failure" boolean to true in an "otherwise" chain. But then step 8 breaks the otherwise chain with a simple "If failure is true, then ...". Then we have a new set of otherwise statements that attempt to access the response of navigationParams. However, if navigationParams is a non-fetch scheme navigation params, it doesn't have a response.

I assume that after executing step 3, we are supposed to skip down to steps 13 and 14 to set the "ever populated" flag on the document, run the completion steps, and bail from the algorithm.

I'm not sure what a nice way to structure this is, but it seems to me like the current structure doesn't work because of the break in the if/else chain with step 8.

@ADKaster
Copy link
Contributor Author

Related, in step 12 of attempt to populate the history entry's document what is request?

If entry's document state's request referrer is "client", then set it to request's referrer.

It seems out of place. Step 13 checks if entry's document state is non-null before manipulating it. As far as I can tell, there is no request in this algorithm at all.

@domenic domenic self-assigned this Nov 1, 2023
domenic added a commit that referenced this issue Nov 1, 2023
...largely by rewriting everything that takes place inside the queued task. Compared to the previous version, this one has a clearer if/otherwise branching structure, avoids any early returns, and makes it easier to follow when exactly the entry's document state's document is set to a non-null value and what impact that has.

This fixes a couple logic bugs where final steps were not getting applied quite correctly for all branches. Closes #9767.
domenic added a commit that referenced this issue Jan 11, 2024
...largely by rewriting everything that takes place inside the queued task. Compared to the previous version, this one has a clearer if/otherwise branching structure, avoids any early returns, and makes it easier to follow when exactly the entry's document state's document is set to a non-null value and what impact that has.

This fixes a couple logic bugs where final steps were not getting applied quite correctly for all branches. Closes #9767.
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.

3 participants
@domenic @ADKaster and others