fix(common): Prefer to use pageXOffset / pageYOffset instance of scro… · angular/angular@5692607

4 files changed

lines changed

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

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

}

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!);

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