Skip to content

Commit 5692607

Browse files
wKozaAndrewKushnir
authored andcommitted
fix(common): Prefer to use pageXOffset / pageYOffset instance of scrollX / scrollY (#28262)
This fix ensures a better cross-browser compatibility. This fix has been used for angular.io. PR Close #28262
1 parent 2506c02 commit 5692607

File tree

4 files changed

+13
-15
lines changed

4 files changed

+13
-15
lines changed

Diff for: aio/src/app/shared/scroll.service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class ScrollService implements OnDestroy {
2424
poppedStateScrollPosition: ScrollPosition|null = null;
2525
// Whether the browser supports the necessary features for manual scroll restoration.
2626
supportManualScrollRestoration: boolean = !!window && ('scrollTo' in window) &&
27-
('scrollX' in window) && ('scrollY' in window) && isScrollRestorationWritable();
27+
('pageXOffset' in window) && isScrollRestorationWritable();
2828

2929
// Offset from the top of the document to bottom of any static elements
3030
// at the top (e.g. toolbar) + some margin

Diff for: packages/common/src/viewport_scroller.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export class BrowserViewportScroller implements ViewportScroller {
8989
*/
9090
getScrollPosition(): [number, number] {
9191
if (this.supportsScrolling()) {
92-
return [this.window.scrollX, this.window.scrollY];
92+
return [this.window.pageXOffset, this.window.pageYOffset];
9393
} else {
9494
return [0, 0];
9595
}
@@ -149,7 +149,7 @@ export class BrowserViewportScroller implements ViewportScroller {
149149
*/
150150
private supportScrollRestoration(): boolean {
151151
try {
152-
if (!this.window || !this.window.scrollTo) {
152+
if (!this.supportsScrolling()) {
153153
return false;
154154
}
155155
// The `scrollRestoration` property could be on the `history` instance or its prototype.
@@ -166,7 +166,7 @@ export class BrowserViewportScroller implements ViewportScroller {
166166

167167
private supportsScrolling(): boolean {
168168
try {
169-
return !!this.window.scrollTo;
169+
return !!this.window && !!this.window.scrollTo && 'pageXOffset' in this.window;
170170
} catch {
171171
return false;
172172
}

Diff for: packages/common/test/viewport_scroller_spec.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ describe('BrowserViewportScroller', () => {
1515
let windowSpy: any;
1616

1717
beforeEach(() => {
18-
windowSpy = jasmine.createSpyObj('window', ['history', 'scrollTo']);
18+
windowSpy =
19+
jasmine.createSpyObj('window', ['history', 'scrollTo', 'pageXOffset', 'pageYOffset']);
1920
windowSpy.history.scrollRestoration = 'auto';
2021
documentSpy = jasmine.createSpyObj('document', ['getElementById', 'getElementsByName']);
2122
scroller = new BrowserViewportScroller(documentSpy, windowSpy, null!);

Diff for: packages/router/test/bootstrap.spec.ts

+7-10
Original file line numberDiff line numberDiff line change
@@ -348,27 +348,24 @@ describe('bootstrap', () => {
348348
await router.navigateByUrl('/aa');
349349
window.scrollTo(0, 5000);
350350

351-
// IE 11 uses non-standard pageYOffset instead of scrollY
352-
const getScrollY = () => window.scrollY !== undefined ? window.scrollY : window.pageYOffset;
353-
354351
await router.navigateByUrl('/fail');
355-
expect(getScrollY()).toEqual(5000);
352+
expect(window.pageYOffset).toEqual(5000);
356353

357354
await router.navigateByUrl('/bb');
358355
window.scrollTo(0, 3000);
359356

360-
expect(getScrollY()).toEqual(3000);
357+
expect(window.pageYOffset).toEqual(3000);
361358

362359
await router.navigateByUrl('/cc');
363-
expect(getScrollY()).toEqual(0);
360+
expect(window.pageYOffset).toEqual(0);
364361

365362
await router.navigateByUrl('/aa#marker2');
366-
expect(getScrollY()).toBeGreaterThanOrEqual(5900);
367-
expect(getScrollY()).toBeLessThan(6000); // offset
363+
expect(window.pageYOffset).toBeGreaterThanOrEqual(5900);
364+
expect(window.pageYOffset).toBeLessThan(6000); // offset
368365

369366
await router.navigateByUrl('/aa#marker3');
370-
expect(getScrollY()).toBeGreaterThanOrEqual(8900);
371-
expect(getScrollY()).toBeLessThan(9000);
367+
expect(window.pageYOffset).toBeGreaterThanOrEqual(8900);
368+
expect(window.pageYOffset).toBeLessThan(9000);
372369
done();
373370
});
374371

0 commit comments

Comments
 (0)