Skip to content

Commit 2717416

Browse files
atscottmmalerba
authored andcommitted
refactor(core): Update push/replace navigation to not trigger popstate (#60028)
This matches the spec, though there is a bug in chrome that does trigger these events. PR Close #60028
1 parent bf89792 commit 2717416

File tree

2 files changed

+52
-68
lines changed

2 files changed

+52
-68
lines changed

packages/core/primitives/dom-navigation/testing/fake_navigation.ts

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ export class FakeNavigation implements Navigation {
232232
// Always false for pushState() or replaceState().
233233
userInitiated: false,
234234
hashChange,
235-
triggeredByHistoryApi: true,
236235
});
237236
}
238237

@@ -456,7 +455,6 @@ export class FakeNavigation implements Navigation {
456455
destination,
457456
info: options.info,
458457
sameDocument: destination.sameDocument,
459-
triggeredByHistoryApi: options.triggeredByHistoryApi,
460458
result,
461459
});
462460
}
@@ -479,34 +477,14 @@ export class FakeNavigation implements Navigation {
479477
}
480478
// "If navigationType is "push" or "replace", then run the URL and history update steps given document and event's destination's URL, with serialiedData set to event's classic history API state and historyHandling set to navigationType."
481479
if (navigateEvent.navigationType === 'push' || navigateEvent.navigationType === 'replace') {
482-
// TODO(atscott): The spec doesn't have this branch and only does urlAndHistoryUpdateSteps
483-
// but I cannot find where popstate
484-
if (navigateEvent.triggeredByHistoryApi) {
485-
this.urlAndHistoryUpdateSteps(navigateEvent);
486-
} else {
487-
this.updateDocumentForHistoryStepApplication(navigateEvent);
488-
}
480+
this.urlAndHistoryUpdateSteps(navigateEvent);
489481
} else if (navigateEvent.navigationType === 'reload') {
490482
this.updateNavigationEntriesForSameDocumentNavigation(navigateEvent);
491483
} else if (navigateEvent.navigationType === 'traverse') {
492484
// "If navigationType is "traverse", then this event firing is happening as part of the traversal process, and that process will take care of performing the appropriate session history entry updates."
493485
}
494486
}
495487

496-
/**
497-
* https://html.spec.whatwg.org/multipage/browsing-the-web.html#update-document-for-history-step-application
498-
* @param navigateEvent
499-
*/
500-
private updateDocumentForHistoryStepApplication(navigateEvent: InternalFakeNavigateEvent) {
501-
this.updateNavigationEntriesForSameDocumentNavigation(navigateEvent);
502-
// Happens as part of "updating the document" steps https://whatpr.org/html/10919/browsing-the-web.html#updating-the-document
503-
const popStateEvent = createPopStateEvent({
504-
state: navigateEvent.destination.getHistoryState(),
505-
});
506-
this.window.dispatchEvent(popStateEvent);
507-
// TODO(atscott): If oldURL's fragment is not equal to entry's URL's fragment, then queue a global task to fire an event named hashchange
508-
}
509-
510488
/**
511489
* Implementation for a push or replace navigation.
512490
* https://whatpr.org/html/10919/browsing-the-web.html#url-and-history-update-steps
@@ -533,6 +511,7 @@ export class FakeNavigation implements Navigation {
533511
state: navigateEvent.destination.getHistoryState(),
534512
});
535513
this.window.dispatchEvent(popStateEvent);
514+
// TODO(atscott): If oldURL's fragment is not equal to entry's URL's fragment, then queue a global task to fire an event named hashchange
536515
}
537516

538517
/** https://whatpr.org/html/10919/nav-history-apis.html#update-the-navigation-api-entries-for-a-same-document-navigation */
@@ -770,10 +749,10 @@ export interface FakeNavigateEvent extends ExperimentalNavigateEvent {
770749

771750
interface InternalFakeNavigateEvent extends FakeNavigateEvent {
772751
readonly sameDocument: boolean;
773-
readonly triggeredByHistoryApi?: boolean;
774752
readonly commitOption: 'after-transition' | 'immediate';
775753
readonly result: InternalNavigationResult;
776-
interceptionState: 'none' | 'intercepted' | 'committed' | 'scrolled' | 'finished';
754+
// TODO(atscott): rejected is not in the spec https://github.com/whatwg/html/issues/11087
755+
interceptionState: 'none' | 'intercepted' | 'committed' | 'scrolled' | 'finished' | 'rejected';
777756
scrollBehavior: 'after-transition' | 'manual' | null;
778757
focusResetBehavior: 'after-transition' | 'manual' | null;
779758

@@ -797,7 +776,6 @@ function dispatchNavigateEvent({
797776
destination,
798777
info,
799778
sameDocument,
800-
triggeredByHistoryApi,
801779
result,
802780
}: {
803781
cancelable: boolean;
@@ -809,7 +787,6 @@ function dispatchNavigateEvent({
809787
destination: FakeNavigationDestination;
810788
info: unknown;
811789
sameDocument: boolean;
812-
triggeredByHistoryApi?: boolean;
813790
result: InternalNavigationResult;
814791
}) {
815792
const {navigation} = result;
@@ -831,7 +808,6 @@ function dispatchNavigateEvent({
831808
event.result = result;
832809

833810
event.sameDocument = sameDocument;
834-
event.triggeredByHistoryApi = triggeredByHistoryApi;
835811
event.commitOption = 'immediate';
836812

837813
let handlersFinished = [Promise.resolve()];
@@ -936,7 +912,9 @@ function dispatchNavigateEvent({
936912
const navigatesuccessEvent = new Event('navigatesuccess', {bubbles: false, cancelable});
937913
navigation.eventTarget.dispatchEvent(navigatesuccessEvent);
938914
result.finishedResolve();
939-
// TODO(atscott): If navigation's transition is not null, then resolve navigation's transition's finished promise with undefined.
915+
if (navigation.transition !== null) {
916+
(navigation.transition as InternalNavigationTransition).finishedResolve();
917+
}
940918
navigation.transition = null;
941919
},
942920
(reason) => {
@@ -947,11 +925,14 @@ function dispatchNavigateEvent({
947925
throw new Error("Navigation's ongoing event not equal to resolved event");
948926
}
949927
navigation.navigateEvent = null;
928+
event.interceptionState = 'rejected'; // TODO(atscott): this is not in the spec https://github.com/whatwg/html/issues/11087
950929
finishNavigationEvent(event, false);
951930
const navigateerrorEvent = new Event('navigateerror', {bubbles: false, cancelable});
952931
navigation.eventTarget.dispatchEvent(navigateerrorEvent);
953932
result.finishedReject(reason);
954-
// TODO(atscott): If navigation's transition is not null, then resolve navigation's transition's finished promise with undefined.
933+
if (navigation.transition !== null) {
934+
(navigation.transition as InternalNavigationTransition).finishedResolve();
935+
}
955936
navigation.transition = null;
956937
},
957938
);
@@ -970,8 +951,9 @@ function finishNavigationEvent(event: InternalFakeNavigateEvent, didFulfill: boo
970951
if (event.interceptionState === 'none') {
971952
return;
972953
}
973-
potentiallyResetFocus(event);
974954
if (didFulfill) {
955+
// TODO(atscott): https://github.com/whatwg/html/issues/11087 focus reset is not guarded by didFulfill in the spec
956+
potentiallyResetFocus(event);
975957
potentiallyResetScroll(event);
976958
}
977959
event.interceptionState = 'finished';
@@ -1144,7 +1126,14 @@ class InternalNavigationResult {
11441126
});
11451127

11461128
this.finished = new Promise<FakeNavigationHistoryEntry>(async (resolve, reject) => {
1147-
this.finishedResolve = () => void resolve(this.committedTo!);
1129+
this.finishedResolve = () => {
1130+
if (this.committedTo === null) {
1131+
throw new Error(
1132+
'NavigateEvent should have been committed before resolving finished promise.',
1133+
);
1134+
}
1135+
resolve(this.committedTo);
1136+
};
11481137
this.finishedReject = (reason: Error) => {
11491138
reject(reason);
11501139
this.abortController.abort(reason);
@@ -1164,5 +1153,4 @@ interface InternalNavigateOptions {
11641153
userInitiated: boolean;
11651154
hashChange: boolean;
11661155
info?: unknown;
1167-
triggeredByHistoryApi?: boolean;
11681156
}

0 commit comments

Comments
 (0)