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 bb88c9fa3daac80086efbda951d81c159e3840f4, 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
This commit is contained in:
		
							parent
							
								
									ca798804b2
								
							
						
					
					
						commit
						71079ce47e
					
				| @ -88,7 +88,7 @@ export class BrowserViewportScroller implements ViewportScroller { | |||||||
|    * @returns The position in screen coordinates. |    * @returns The position in screen coordinates. | ||||||
|    */ |    */ | ||||||
|   getScrollPosition(): [number, number] { |   getScrollPosition(): [number, number] { | ||||||
|     if (this.supportScrollRestoration()) { |     if (this.supportsScrolling()) { | ||||||
|       return [this.window.scrollX, this.window.scrollY]; |       return [this.window.scrollX, this.window.scrollY]; | ||||||
|     } else { |     } else { | ||||||
|       return [0, 0]; |       return [0, 0]; | ||||||
| @ -100,7 +100,7 @@ export class BrowserViewportScroller implements ViewportScroller { | |||||||
|    * @param position The new position in screen coordinates. |    * @param position The new position in screen coordinates. | ||||||
|    */ |    */ | ||||||
|   scrollToPosition(position: [number, number]): void { |   scrollToPosition(position: [number, number]): void { | ||||||
|     if (this.supportScrollRestoration()) { |     if (this.supportsScrolling()) { | ||||||
|       this.window.scrollTo(position[0], position[1]); |       this.window.scrollTo(position[0], position[1]); | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| @ -110,7 +110,7 @@ export class BrowserViewportScroller implements ViewportScroller { | |||||||
|    * @param anchor The ID of the anchor element. |    * @param anchor The ID of the anchor element. | ||||||
|    */ |    */ | ||||||
|   scrollToAnchor(anchor: string): void { |   scrollToAnchor(anchor: string): void { | ||||||
|     if (this.supportScrollRestoration()) { |     if (this.supportsScrolling()) { | ||||||
|       const elSelected = |       const elSelected = | ||||||
|           this.document.getElementById(anchor) || this.document.getElementsByName(anchor)[0]; |           this.document.getElementById(anchor) || this.document.getElementsByName(anchor)[0]; | ||||||
|       if (elSelected) { |       if (elSelected) { | ||||||
| @ -163,6 +163,14 @@ export class BrowserViewportScroller implements ViewportScroller { | |||||||
|       return false; |       return false; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  | 
 | ||||||
|  |   private supportsScrolling(): boolean { | ||||||
|  |     try { | ||||||
|  |       return !!this.window.scrollTo; | ||||||
|  |     } catch { | ||||||
|  |       return false; | ||||||
|  |     } | ||||||
|  |   } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| function getScrollRestorationProperty(obj: any): PropertyDescriptor|undefined { | function getScrollRestorationProperty(obj: any): PropertyDescriptor|undefined { | ||||||
|  | |||||||
| @ -15,21 +15,30 @@ describe('BrowserViewportScroller', () => { | |||||||
|   let windowSpy: any; |   let windowSpy: any; | ||||||
| 
 | 
 | ||||||
|   beforeEach(() => { |   beforeEach(() => { | ||||||
|     windowSpy = jasmine.createSpyObj('window', ['history']); |     windowSpy = jasmine.createSpyObj('window', ['history', 'scrollTo']); | ||||||
|     windowSpy.scrollTo = 1; |  | ||||||
|     windowSpy.history.scrollRestoration = 'auto'; |     windowSpy.history.scrollRestoration = 'auto'; | ||||||
|     documentSpy = jasmine.createSpyObj('document', ['getElementById', 'getElementsByName']); |     documentSpy = jasmine.createSpyObj('document', ['getElementById', 'getElementsByName']); | ||||||
|     scroller = new BrowserViewportScroller(documentSpy, windowSpy, null!); |     scroller = new BrowserViewportScroller(documentSpy, windowSpy, null!); | ||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|   describe('setHistoryScrollRestoration', () => { |   describe('setHistoryScrollRestoration', () => { | ||||||
|     it('should not crash when scrollRestoration is not writable', () => { |     function createNonWritableScrollRestoration() { | ||||||
|       Object.defineProperty(windowSpy.history, 'scrollRestoration', { |       Object.defineProperty(windowSpy.history, 'scrollRestoration', { | ||||||
|         value: 'auto', |         value: 'auto', | ||||||
|         configurable: true, |         configurable: true, | ||||||
|       }); |       }); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     it('should not crash when scrollRestoration is not writable', () => { | ||||||
|  |       createNonWritableScrollRestoration(); | ||||||
|       expect(() => scroller.setHistoryScrollRestoration('manual')).not.toThrow(); |       expect(() => scroller.setHistoryScrollRestoration('manual')).not.toThrow(); | ||||||
|     }); |     }); | ||||||
|  | 
 | ||||||
|  |     it('should still allow scrolling if scrollRestoration is not writable', () => { | ||||||
|  |       createNonWritableScrollRestoration(); | ||||||
|  |       scroller.scrollToPosition([10, 10]); | ||||||
|  |       expect(windowSpy.scrollTo as jasmine.Spy).toHaveBeenCalledWith(10, 10); | ||||||
|  |     }); | ||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|   describe('scrollToAnchor', () => { |   describe('scrollToAnchor', () => { | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user