Repository: angular
Issue: Scroll position is not restored when skipLocationChange is used in a page gaurd
Labels: area: router

### Which @angular/* package(s) are the source of the bug?

router

### Is this a regression?

No

### Description

When `skipLocationChange` is used in a page guard, the scroll position is not restored on back navigation.

I believe this occurs due to how angular's `RouterScroller` stores the position for `Scroll` events:

```ts
private createScrollEvents() {
    return this.transitions.events.subscribe(e => {
      if (e instanceof NavigationStart) {
        // store the scroll position of the current stable navigations.
        this.store[this.lastId] = this.viewportScroller.getScrollPosition();
        this.lastSource = e.navigationTrigger;
        this.restoredId = e.restoredState ? e.restoredState.navigationId : 0;
      } else if (e instanceof NavigationEnd) {
        this.lastId = e.id;
        this.scheduleScrollEvent(e, this.urlSerializer.parse(e.urlAfterRedirects).fragment);
      } else if (
          e instanceof NavigationSkipped &&
          e.code === NavigationSkippedCode.IgnoredSameUrlNavigation) {
        this.lastSource = undefined;
        this.restoredId = 0;
        this.scheduleScrollEvent(e, this.urlSerializer.parse(e.url).fragment);
      }
    });
  }
```

On `NavigationStart` events we set the `lastSource` to be the `navigationTrigger` of the `NavigationStart` event and then on `NavigationEnd` events we schedule a scroll event and then in `scheduleScrollEvent` we only create a Scroll event with the position if the `lastSource` was `popstate`.

```ts
private scheduleScrollEvent(routerEvent: NavigationEnd|NavigationSkipped, anchor: string|null):
    void {
  this.zone.runOutsideAngular(() => {
    // The scroll event needs to be delayed until after change detection. Otherwise, we may
    // attempt to restore the scroll position before the router outlet has fully rendered the
    // component by executing its update block of the template function.
    setTimeout(() => {
      this.zone.run(() => {
        this.transitions.events.next(new Scroll(
            routerEvent, this.lastSource === 'popstate' ? this.store[this.restoredId] : null,
            anchor));
      });
    }, 0);
  });
}
```

This causes the problem when `skipLocationChange` is used in a page guard as the sequence of navigation events when using the back navigation is:

![Screenshot from 2023-05-16 16-24-59](https://github.com/angular/angular/assets/58979424/d29ad877-126d-4588-ae6b-f34bd2d7aef1)
_(Screenshot from minimal reproduction)_


The initial `NavigationStart` event has a navigation trigger of `popstate` but the redirect in the guard from `/` to `/internal` has a navigation trigger of `imperative`, which results in the `lastSource` being `imperative` when `scheuleScrollEvent` is called, resulting in the `Scroll` event having `undefined` for its position, due to the line:

```ts
this.lastSource === 'popstate' ? this.store[this.restoredId] : null
```

### Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/angular-rkjuvp

### Please provide the exception or error you saw

```true
When a user navigates back to the previous page they are scrolled to the top instead of being scrolled to restored position.
```


### Please provide the environment you discovered this bug in (run `ng version`)

```true
Angular CLI: 14.2.10
Node: 16.20.0
Package Manager: yarn 1.22.19 
OS: linux x64

Angular: 14.2.12
... animations, common, compiler, compiler-cli, core, forms
... language-service, localize, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1402.10
@angular-devkit/build-angular   14.2.10
@angular-devkit/core            14.2.10
@angular-devkit/schematics      14.2.10
@angular/cdk                    14.2.5
@angular/cli                    14.2.10
@angular/fire                   7.4.1
@angular/material               14.2.5
@nguniversal/builders           14.2.3
@nguniversal/express-engine     14.0.3
@schematics/angular             14.2.10
rxjs                            7.5.7
typescript                      4.8.4
```


### Anything else?

Here is a video for reference:
[angular scroll video.webm](https://github.com/angular/angular/assets/58979424/a379c682-a267-4fc8-a9d7-9ed8cf6f6fc8)

One workaround we were thinking of is to store the last event in the `RouterScroller` and if the current event was an instance of `NavigationStart`, the last event was also an instance of `NavigationStart` and the last source was `popstate`, we assume the current `NavigationStart` event was triggered by a `skipLocationChange` in a page guard and don't update the `lastSource`, however this wouldn't account for the edge case of having a guard which can redirect to multiple possible pages with `skipLocationChange` - Not sure what would be the best approach for handling that case. 

```diff
@@ -6427,6 +6427,7 @@ class RouterScroller {
         this.options = options;
+        this.lastEvent = undefined;
         this.lastId = 0;
         this.lastSource = 'imperative';
         this.restoredId = 0;
         this.store = {};
         // Default both options to 'disabled'
@@ -6445,16 +6446,32 @@ class RouterScroller {
     }
     createScrollEvents() {
         return this.router.events.subscribe(e => {
+            if (!(e instanceof NavigationStart || e instanceof NavigationEnd)) {
+                return;
+            }
+
             if (e instanceof NavigationStart) {
                 // store the scroll position of the current stable navigations.
                 this.store[this.lastId] = this.viewportScroller.getScrollPosition();
-                this.lastSource = e.navigationTrigger;
-                this.restoredId = e.restoredState ? e.restoredState.navigationId : 0;
+                if (
+                    this.router.getCurrentNavigation().extras.skipLocationChange &&
+                    this.lastEvent instanceof NavigationStart &&
+                    this.lastSource === 'popstate'
+                ) {
+                    // This is the case where a skipLocationChange redirect was used in the page guard. We need to increment `restoredId`
+                    // here as the restoredState is null for this `NavigationStart` event as it is classified an imperative navigation
+                    this.restoredId++;
+                } else {
+                    this.lastSource = e.navigationTrigger;
+                    this.restoredId = e.restoredState ? e.restoredState.navigationId : 0;
+                }
             }
             else if (e instanceof NavigationEnd) {
                 this.lastId = e.id;
                 this.scheduleScrollEvent(e, this.router.parseUrl(e.urlAfterRedirects).fragment);
             }
+
+            this.lastEvent = e;
         });
     }
     consumeScrollEvents() {
```