Fire navigate events on all windows affected during traversal #129
Conversation
|
|
||
| 1. Let |destinationURL| be |destinationEntry|'s [=session history entry/URL=]. | ||
| 1. Let |destinationState| be |destinationEntry|'s [=session history entry/app history state=]. | ||
| 1. Let |isSameDocument| be true if |destinationEntry|'s [=session history entry/document=] is equal to |appHistory|'s [=relevant global object=]'s [=associated Document=]; otherwise false. |
There was a problem hiding this comment.
Eventually it might be best to compare the "document state" between the active entry and the destination entry, as this caters for cases where the document is null.
Although, I guess conveniently, the document will never be null in this case.
| <div algorithm="apply the history step"> | ||
| Modify the <a spec="HTML">apply the history step</a> algorithm as follows. Change <var ignore>checkForUserCancellation</var> to |fireBeforeunloadAndNavigate|. Add the |userInvolvement| parameter. Then, insert the following step after step 12 (which assembles |toTraverse| and other lists) but before step 13 (which checks if unloading is user-cancelled): | ||
|
|
||
| 1. If |fireBeforeunloadAndNavigate| is true and the result of <a>firing traversal `navigate` events</a> given |toTraverse|, <var ignore>step</var>, <var ignore>initiatorToCheck</var>, and |userInvolvement| is false, then return. |
There was a problem hiding this comment.
Nit: It seems weird to me when an algorithm returns a boolean like this, but the name of the algorithm doesn't suggest what the return means. That's why I went for "checking if unloading is user-cancelled", but I guess the flip side of that is it isn't clear that the algorithm also fires events.
There was a problem hiding this comment.
Yeah, I agree it's a bit iffy but I'm modeling it after the existing "fire an event" algorithm.
| 1. Let |completedTasks| be 0. | ||
| 1. [=list/For each=] |navigable| of |toTraverse|, [=queue a global task=] on the [=history traversal task source=] given |navigable|'s [=navigable/active document=]'s [=relevant global object=] to run these steps: | ||
| 1. Let |destinationEntry| be the item in the result of [=navigable/getting the session history entries=] for |navigable| that has the greatest [=session history entry/step=] less than or equal to |step|. | ||
| 1. If |destinationEntry|'s [=session history entry/document=] is not equal to |navigable|'s [=navigable/active document=], and |initiatorOrigin| is not [=same origin-domain=] with |navigable|'s [=navigable/active document=]'s [=Document/origin=], then abort these steps. |
There was a problem hiding this comment.
Again, this could compare document states rather than documents, but it doesn't really matter.
It would matter in cases where both documents could be null, which would be unintentionally equal.
There was a problem hiding this comment.
A navigable's active document is never null, right?
|
@natechapin, now I'd like your review. This doesn't yet account for the complexities of goTo() around passing info and dealing with the promise return value. However it shows the architecture that we'd use as a base. Roughly, any history traversal (whether via
I realize this isn't how we currently implement things, and might take some work to get there. In particular, currently we don't yet support traversing multiple frames at once (due to "the bug"), and once we do, we probably need extra code to handle beforeunload in multiple frames, so I imagine firing navigate in multiple frames might also be difficult. But does it seem directionally reasonable? @csreis might also be good to review the above flow, although I'm not sure if he wants to review the actual spec text. |
Closes #114. @jakearchibald, can you review and also help with the TODO on figuring out the right step?
Preview | Diff