Skip to content

Commit b32126c

Browse files
committed
fix(common): Allow scrolling when browser supports scrollTo (#38468)
This commit fixes a regression from "fix(common): ensure scrollRestoration is writable (#30630)" that caused scrolling to not happen at all in browsers that do not support scroll restoration. The issue was that `supportScrollRestoration` was updated to return `false` if a browser did not have a writable `scrollRestoration`. However, the previous behavior was that the function would return `true` if `window.scrollTo` was defined. Every scrolling function in the `ViewportScroller` used `supportScrollRestoration` and, with the update in bb88c9f, no scrolling would be performed if a browser did not have writable `scrollRestoration` but _did_ have `window.scrollTo`. Note, that this failure was detected in the saucelabs tests. IE does not support scroll restoration so IE tests were failing. PR Close #38468
1 parent d5e09f4 commit b32126c

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

packages/common/src/viewport_scroller.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export class BrowserViewportScroller implements ViewportScroller {
8888
* @returns The position in screen coordinates.
8989
*/
9090
getScrollPosition(): [number, number] {
91-
if (this.supportScrollRestoration()) {
91+
if (this.supportsScrolling()) {
9292
return [this.window.scrollX, this.window.scrollY];
9393
} else {
9494
return [0, 0];
@@ -100,7 +100,7 @@ export class BrowserViewportScroller implements ViewportScroller {
100100
* @param position The new position in screen coordinates.
101101
*/
102102
scrollToPosition(position: [number, number]): void {
103-
if (this.supportScrollRestoration()) {
103+
if (this.supportsScrolling()) {
104104
this.window.scrollTo(position[0], position[1]);
105105
}
106106
}
@@ -110,7 +110,7 @@ export class BrowserViewportScroller implements ViewportScroller {
110110
* @param anchor The ID of the anchor element.
111111
*/
112112
scrollToAnchor(anchor: string): void {
113-
if (this.supportScrollRestoration()) {
113+
if (this.supportsScrolling()) {
114114
const elSelected =
115115
this.document.getElementById(anchor) || this.document.getElementsByName(anchor)[0];
116116
if (elSelected) {
@@ -163,6 +163,14 @@ export class BrowserViewportScroller implements ViewportScroller {
163163
return false;
164164
}
165165
}
166+
167+
private supportsScrolling(): boolean {
168+
try {
169+
return !!this.window.scrollTo;
170+
} catch {
171+
return false;
172+
}
173+
}
166174
}
167175

168176
function getScrollRestorationProperty(obj: any): PropertyDescriptor|undefined {

packages/common/test/viewport_scroller_spec.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,30 @@ describe('BrowserViewportScroller', () => {
1515
let windowSpy: any;
1616

1717
beforeEach(() => {
18-
windowSpy = jasmine.createSpyObj('window', ['history']);
19-
windowSpy.scrollTo = 1;
18+
windowSpy = jasmine.createSpyObj('window', ['history', 'scrollTo']);
2019
windowSpy.history.scrollRestoration = 'auto';
2120
documentSpy = jasmine.createSpyObj('document', ['getElementById', 'getElementsByName']);
2221
scroller = new BrowserViewportScroller(documentSpy, windowSpy, null!);
2322
});
2423

2524
describe('setHistoryScrollRestoration', () => {
26-
it('should not crash when scrollRestoration is not writable', () => {
25+
function createNonWritableScrollRestoration() {
2726
Object.defineProperty(windowSpy.history, 'scrollRestoration', {
2827
value: 'auto',
2928
configurable: true,
3029
});
30+
}
31+
32+
it('should not crash when scrollRestoration is not writable', () => {
33+
createNonWritableScrollRestoration();
3134
expect(() => scroller.setHistoryScrollRestoration('manual')).not.toThrow();
3235
});
36+
37+
it('should still allow scrolling if scrollRestoration is not writable', () => {
38+
createNonWritableScrollRestoration();
39+
scroller.scrollToPosition([10, 10]);
40+
expect(windowSpy.scrollTo as jasmine.Spy).toHaveBeenCalledWith(10, 10);
41+
});
3342
});
3443

3544
describe('scrollToAnchor', () => {

0 commit comments

Comments
 (0)