From 2848f0499ffe59b9d07c80be83d546d9421e9fcf Mon Sep 17 00:00:00 2001 From: Ward Bell Date: Sat, 6 May 2017 12:30:34 -0700 Subject: [PATCH] fix(aio): do not nav to an image url closes #16608 Formerly, tried to navigate when user clicked an anchor with an image url (to view image in a new tab) resulting in 404. Now ignores href URL with any extension and lets browser handle it. --- aio/src/app/app.component.ts | 4 +- aio/src/app/shared/location.service.spec.ts | 109 +++++++++++++------- aio/src/app/shared/location.service.ts | 23 +++-- 3 files changed, 93 insertions(+), 43 deletions(-) diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts index 76c1bb2f52..b49f4d431b 100644 --- a/aio/src/app/app.component.ts +++ b/aio/src/app/app.component.ts @@ -188,8 +188,8 @@ export class AppComponent implements OnInit { while (target && !(target instanceof HTMLAnchorElement)) { target = target.parentElement; } - if (target) { - return this.locationService.handleAnchorClick(target as HTMLAnchorElement, button, ctrlKey, metaKey); + if (target instanceof HTMLAnchorElement) { + return this.locationService.handleAnchorClick(target, button, ctrlKey, metaKey); } return true; } diff --git a/aio/src/app/shared/location.service.spec.ts b/aio/src/app/shared/location.service.spec.ts index c48f7eab63..a17921250a 100644 --- a/aio/src/app/shared/location.service.spec.ts +++ b/aio/src/app/shared/location.service.spec.ts @@ -296,6 +296,7 @@ describe('LocationService', () => { service.go('https://some/far/away/land'); expect(localUrl).toBeFalsy('should not set local url'); }); + }); describe('search', () => { @@ -378,118 +379,156 @@ describe('LocationService', () => { beforeEach(() => { anchor = document.createElement('a'); + spyOn(service, 'go'); }); - describe('intercepting', () => { - it('should intercept clicks on anchors for relative local urls', () => { + describe('should try to navigate with go() when anchor clicked for', () => { + it('relative local url', () => { anchor.href = 'some/local/url'; - spyOn(service, 'go'); - const result = service.handleAnchorClick(anchor, 0, false, false); + const result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url'); expect(result).toBe(false); }); - it('should intercept clicks on anchors for absolute local urls', () => { + it('absolute local url', () => { anchor.href = '/some/local/url'; - spyOn(service, 'go'); - const result = service.handleAnchorClick(anchor, 0, false, false); + const result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url'); expect(result).toBe(false); }); - it('should intercept clicks on anchors for local urls, with query params', () => { + it('local url with query params', () => { anchor.href = 'some/local/url?query=xxx&other=yyy'; - spyOn(service, 'go'); - const result = service.handleAnchorClick(anchor, 0, false, false); + const result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url?query=xxx&other=yyy'); expect(result).toBe(false); }); - it('should intercept clicks on anchors for local urls, with hash fragment', () => { + it('local url with hash fragment', () => { anchor.href = 'some/local/url#somefragment'; - spyOn(service, 'go'); - const result = service.handleAnchorClick(anchor, 0, false, false); + const result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url#somefragment'); expect(result).toBe(false); }); - it('should intercept clicks on anchors for local urls, with query params and hash fragment', () => { + it('local url with query params and hash fragment', () => { anchor.href = 'some/local/url?query=xxx&other=yyy#somefragment'; - spyOn(service, 'go'); - const result = service.handleAnchorClick(anchor, 0, false, false); + const result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url?query=xxx&other=yyy#somefragment'); expect(result).toBe(false); }); + + it('local url with period in a path segment but no extension', () => { + anchor.href = 'tut.or.ial/toh-p2'; + const result = service.handleAnchorClick(anchor); + expect(service.go).toHaveBeenCalled(); + expect(result).toBe(false); + }); }); - describe('not intercepting', () => { - it('should not intercept clicks on anchors for external urls', () => { + describe('should let browser handle anchor click when', () => { + it('url is external to the site', () => { anchor.href = 'http://other.com/some/local/url?query=xxx&other=yyy#somefragment'; - spyOn(service, 'go'); - let result = service.handleAnchorClick(anchor, 0, false, false); + let result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); anchor.href = 'some/local/url.pdf'; anchor.protocol = 'ftp'; - result = service.handleAnchorClick(anchor, 0, false, false); + result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); }); - it('should not intercept clicks on anchors if button is not zero', () => { + it('mouse button is not zero (middle or right)', () => { anchor.href = 'some/local/url'; - spyOn(service, 'go'); const result = service.handleAnchorClick(anchor, 1, false, false); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); }); - it('should not intercept clicks on anchors if ctrl key is pressed', () => { + it('ctrl key is pressed', () => { anchor.href = 'some/local/url'; - spyOn(service, 'go'); const result = service.handleAnchorClick(anchor, 0, true, false); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); }); - it('should not intercept clicks on anchors if meta key is pressed', () => { + it('meta key is pressed', () => { anchor.href = 'some/local/url'; - spyOn(service, 'go'); const result = service.handleAnchorClick(anchor, 0, false, true); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); }); - it('should not intercept clicks on links with (non-_self) targets', () => { + it('anchor has (non-_self) target', () => { anchor.href = 'some/local/url'; - spyOn(service, 'go'); - anchor.target = '_blank'; - let result = service.handleAnchorClick(anchor, 0, false, false); + let result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); anchor.target = '_parent'; - result = service.handleAnchorClick(anchor, 0, false, false); + result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); anchor.target = '_top'; - result = service.handleAnchorClick(anchor, 0, false, false); + result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); anchor.target = 'other-frame'; - result = service.handleAnchorClick(anchor, 0, false, false); + result = service.handleAnchorClick(anchor); expect(service.go).not.toHaveBeenCalled(); expect(result).toBe(true); anchor.target = '_self'; - result = service.handleAnchorClick(anchor, 0, false, false); + result = service.handleAnchorClick(anchor); expect(service.go).toHaveBeenCalledWith('/some/local/url'); expect(result).toBe(false); }); + + it('zip url', () => { + anchor.href = 'tutorial/toh-p2.zip'; + const result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('image or media url', () => { + anchor.href = 'cat-photo.png'; + let result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true, 'png'); + + anchor.href = 'cat-photo.gif'; + result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true, 'gif'); + + anchor.href = 'cat-photo.jpg'; + result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true, 'jpg'); + + anchor.href = 'dog-bark.mp3'; + result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true, 'mp3'); + + anchor.href = 'pet-tricks.mp4'; + result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true, 'mp4'); + }); + + it('url has any extension', () => { + anchor.href = 'tutorial/toh-p2.html'; + const result = service.handleAnchorClick(anchor); + expect(service.go).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); }); }); diff --git a/aio/src/app/shared/location.service.ts b/aio/src/app/shared/location.service.ts index 2b7e755285..e88bc2cc57 100644 --- a/aio/src/app/shared/location.service.ts +++ b/aio/src/app/shared/location.service.ts @@ -87,6 +87,14 @@ export class LocationService { } /** + * Handle user's anchor click + * + * @param anchor {HTMLAnchorElement} - the anchor element clicked + * @param button Number of the mouse button held down. 0 means left or none + * @param ctrlKey True if control key held down + * @param metaKey True if command or window key held down + * @return false if service navigated with `go()`; true if browser should handle it. + * * Since we are using `LocationService` to navigate between docs, without the browser * reloading the page, we must intercept clicks on links. * If the link is to a document that we will render, then we navigate using `Location.go()` @@ -99,9 +107,10 @@ export class LocationService { * `AppComponent`, whose element contains all the of the application and so captures all * link clicks both inside and outside the `DocViewerComponent`. */ - handleAnchorClick(anchor: HTMLAnchorElement, button: number, ctrlKey: boolean, metaKey: boolean) { - // Check for modifier keys, which indicate the user wants to control navigation + handleAnchorClick(anchor: HTMLAnchorElement, button = 0, ctrlKey = false, metaKey = false) { + + // Check for modifier keys and non-left-button, which indicate the user wants to control navigation if (button !== 0 || ctrlKey || metaKey) { return true; } @@ -114,19 +123,21 @@ export class LocationService { return true; } - // don't navigate if external link or zip - const { pathname, search, hash } = anchor; - if (anchor.getAttribute('download') != null) { return true; // let the download happen } + const { pathname, search, hash } = anchor; const relativeUrl = pathname + search + hash; this.urlParser.href = relativeUrl; - if (anchor.href !== this.urlParser.href) { + + // don't navigate if external link or has extension + if ( anchor.href !== this.urlParser.href || + !/\/[^/.]*$/.test(pathname) ) { return true; } + // approved for navigation this.go(relativeUrl); return false; }