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

Fix signal specification #130

Closed
wants to merge 1 commit into from
Closed

Fix signal specification #130

wants to merge 1 commit into from

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Jul 16, 2021

Actually implementing this uncovered a few spec bugs. These edits largely mirror the structure in https://chromium-review.googlesource.com/c/chromium/src/+/2877951.


Preview | Diff

Actually implementing this uncovered a few spec bugs. These edits largely mirror the structure in https://chromium-review.googlesource.com/c/chromium/src/+/2877951.
@domenic domenic requested a review from natechapin July 16, 2021 20:52
@domenic
Copy link
Collaborator Author

domenic commented Jul 16, 2021

Per offline discussion I think the spec is more broken than this makes it appear. E.g. the "navigate method call promise" isn't sticking around long enough for us to reject it on aborted navigations, which is bad.

It might be better to build this on top of #109 where the "navigate event side table" gives us a clear place to put the signal.

domenic added a commit that referenced this pull request Jul 23, 2021
This commit introduces a much more robust framework for tracking ongoing navigations. This fixes #130, for example, by appropriately keeping the signal around so that we can signal abort when necessary. It also ensures every same-document navigation ends in either navigationsuccess or navigationerror, with a few unified spec paths.

With this framework in hand, we can specify goTo()/back()/forward().
@domenic
Copy link
Collaborator Author

domenic commented Jul 23, 2021

Rolling into #109.

@domenic domenic closed this Jul 23, 2021
@domenic domenic deleted the ongoing-signal branch July 23, 2021 21:47
domenic added a commit that referenced this pull request Jul 27, 2021
This commit introduces a much more robust framework for tracking ongoing navigations. This fixes #130, for example, by appropriately keeping the signal around so that we can signal abort when necessary. It also ensures every same-document navigation ends in either navigationsuccess or navigationerror, with a few unified spec paths.

With this framework in hand, we can specify goTo()/back()/forward().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant