From f2e39be3acb8b9a45bc8f090296b88dc5df5a6b8 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Thu, 22 Sep 2016 13:00:48 -0700 Subject: [PATCH] PWA: migrate document-click service to ampdoc scope (#5154) * PWA: Migrate document-click to ampdoc scope * minors * fixed tests * liter --- src/amp.js | 4 +- src/document-click.js | 91 +++++++++++--------------- test/functional/test-document-click.js | 62 ++++++++++-------- 3 files changed, 74 insertions(+), 83 deletions(-) diff --git a/src/amp.js b/src/amp.js index 9b5ddaf49e57..998ad5325e33 100644 --- a/src/amp.js +++ b/src/amp.js @@ -21,7 +21,7 @@ import './polyfills'; import {installPerformanceService} from './service/performance-impl'; import {installPullToRefreshBlocker} from './pull-to-refresh'; -import {installGlobalClickListener} from './document-click'; +import {installGlobalClickListenerForDoc} from './document-click'; import {installStyles, makeBodyVisible} from './style-installer'; import {installErrorReporting} from './error'; import {installDocService} from './service/ampdoc-impl'; @@ -68,7 +68,7 @@ try { stubElements(self); installPullToRefreshBlocker(self); - installGlobalClickListener(self); + installGlobalClickListenerForDoc(ampdoc); maybeValidate(self); makeBodyVisible(self.document, /* waitForServices */ true); diff --git a/src/document-click.js b/src/document-click.js index 2e94919ede14..9056c32ae584 100644 --- a/src/document-click.js +++ b/src/document-click.js @@ -15,7 +15,7 @@ */ import {closestByTag} from './dom'; -import {fromClass} from './service'; +import {fromClassForDoc} from './service'; import {dev} from './log'; import {historyForDoc} from './history'; import {openWindowDialog} from './dom'; @@ -28,33 +28,15 @@ import {urlReplacementsFor} from './url-replacements'; /** @private @const {string} */ const ORIGINAL_HREF_ATTRIBUTE = 'data-amp-orig-href'; -/** - * @param {!Window} window - */ -export function installGlobalClickListener(window) { - clickHandlerFor(window); - captureClickHandlerFor(window); -} /** - * @param {!Window} window + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc */ -export function uninstallGlobalClickListener(window) { - clickHandlerFor(window).cleanup(); - captureClickHandlerFor(window).cleanup(); +export function installGlobalClickListenerForDoc(ampdoc) { + fromClassForDoc(ampdoc, 'clickhandler', ClickHandler); + fromClassForDoc(ampdoc, 'CaptureClickHandler', CaptureClickHandler); } -/** - * @param {!Window} window - * @return {!ClickHandler} bubble document click handler. - */ -function clickHandlerFor(window) { - return fromClass(window, 'clickhandler', ClickHandler); -} - -function captureClickHandlerFor(window) { - return fromClass(window, 'CaptureClickHandler', CaptureClickHandler); -} /** * Intercept any click on the current document and prevent any @@ -63,22 +45,22 @@ function captureClickHandlerFor(window) { */ export class ClickHandler { /** - * @param {!Window} window + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc */ - constructor(window) { - /** @const {!Window} */ - this.win = window; + constructor(ampdoc) { + /** @const {!./service/ampdoc-impl.AmpDoc} */ + this.ampdoc = ampdoc; /** @private @const {!./service/viewport-impl.Viewport} */ - this.viewport_ = viewportFor(this.win); + this.viewport_ = viewportFor(this.ampdoc.win); /** @private @const {!./service/viewer-impl.Viewer} */ - this.viewer_ = viewerFor(this.win); + this.viewer_ = viewerFor(this.ampdoc.win); /** @private @const {!./service/history-impl.History} */ - this.history_ = historyForDoc(this.win.document); + this.history_ = historyForDoc(this.ampdoc); - const platform = platformFor(this.win); + const platform = platformFor(this.ampdoc.win); /** @private @const {boolean} */ this.isIosSafari_ = platform.isIos() && platform.isSafari(); @@ -86,8 +68,7 @@ export class ClickHandler { if (this.viewer_.isIframed() && this.viewer_.isOvertakeHistory()) { /** @private @const {!function(!Event)|undefined} */ this.boundHandle_ = this.handle_.bind(this); - this.win.document.documentElement.addEventListener( - 'click', this.boundHandle_); + this.ampdoc.getRootNode().addEventListener('click', this.boundHandle_); } } @@ -96,8 +77,7 @@ export class ClickHandler { */ cleanup() { if (this.boundHandle_) { - this.win.document.documentElement.removeEventListener( - 'click', this.boundHandle_); + this.ampdoc.getRootNode().removeEventListener('click', this.boundHandle_); } } @@ -109,10 +89,11 @@ export class ClickHandler { */ handle_(e) { onDocumentElementClick_( - e, this.viewport_, this.history_, this.isIosSafari_); + e, this.ampdoc, this.viewport_, this.history_, this.isIosSafari_); } } + /** * Intercept any click on the current document and prevent any * linking to an identifier from pushing into the history stack. @@ -120,19 +101,19 @@ export class ClickHandler { */ export class CaptureClickHandler { /** - * @param {!Window} window + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc */ - constructor(window) { - /** @const {!Window} */ - this.win = window; + constructor(ampdoc) { + /** @const {!./service/ampdoc-impl.AmpDoc} */ + this.ampdoc = ampdoc; /** @private @const {!./service/url-replacements-impl.UrlReplacements} */ - this.urlReplacements_ = urlReplacementsFor(this.win); + this.urlReplacements_ = urlReplacementsFor(this.ampdoc.win); /** @private {!function(!Event)} */ this.boundHandler_ = this.handle_.bind(this); - this.win.document.documentElement.addEventListener( + this.ampdoc.getRootNode().addEventListener( 'click', this.boundHandler_, true); } @@ -140,8 +121,8 @@ export class CaptureClickHandler { * Removes all event listeners. */ cleanup() { - this.win.document.documentElement.removeEventListener( - 'click', this.boundHandler_); + this.ampdoc.getRootNode().removeEventListener( + 'click', this.boundHandler_, true); } /** @@ -153,6 +134,7 @@ export class CaptureClickHandler { } } + /** * Locate first element with given tag name within event path from shadowRoot. * @param {!Event} e @@ -171,6 +153,7 @@ export function getElementByTagNameFromEventShadowDomPath_(e, tagName) { return null; } + /** * Expands target anchor href on capture click event. If within shadow DOM, * will offset from host element. @@ -191,6 +174,7 @@ export function onDocumentElementCapturedClick_(e, urlReplacements) { } } + /** * Intercept any click on the current document and prevent any * linking to an identifier from pushing into the history stack. @@ -199,11 +183,13 @@ export function onDocumentElementCapturedClick_(e, urlReplacements) { * on iOS Safari. * * @param {!Event} e + * @param {!./service/ampdoc-impl.AmpDoc} ampdoc * @param {!./service/viewport-impl.Viewport} viewport * @param {!./service/history-impl.History} history * @param {boolean} isIosSafari */ -export function onDocumentElementClick_(e, viewport, history, isIosSafari) { +export function onDocumentElementClick_( + e, ampdoc, viewport, history, isIosSafari) { if (e.defaultPrevented) { return; } @@ -213,10 +199,7 @@ export function onDocumentElementClick_(e, viewport, history, isIosSafari) { return; } - const docElement = e.currentTarget; - const doc = docElement.ownerDocument; - const win = doc.defaultView; - + const win = ampdoc.win; const tgtLoc = parseUrl(target.href); // On Safari iOS, custom protocol links will fail to open apps when the @@ -267,11 +250,11 @@ export function onDocumentElementClick_(e, viewport, history, isIosSafari) { const hash = tgtLoc.hash.slice(1); let elem = null; if (hash) { - elem = doc.getElementById(hash); + elem = ampdoc.getRootNode().getElementById(hash); if (!elem) { // Fallback to anchor[name] if element with id is not found. // Linking to an anchor element with name is obsolete in html5. - elem = doc.querySelector(`a[name=${hash}]`); + elem = ampdoc.getRootNode().querySelector(`a[name=${hash}]`); } } @@ -295,7 +278,8 @@ export function onDocumentElementClick_(e, viewport, history, isIosSafari) { history.push(() => { win.location.replace(`${curLoc.hash || '#'}`); }); -}; +} + /** * Get offset location of click from event taking into account shadowRoot. @@ -318,6 +302,7 @@ function getClickLocation_(e) { }; } + /** * Expand click target href synchronously using UrlReplacements service * including CLICK_X/CLICK_Y page offsets (if within shadowRoot will reference @@ -353,4 +338,4 @@ export function expandTargetHref_(e, target, urlReplacements) { target.setAttribute('href', newHref); } return newHref; -}; +} diff --git a/test/functional/test-document-click.js b/test/functional/test-document-click.js index b16dd0f03193..21fdfb2c3ab2 100644 --- a/test/functional/test-document-click.js +++ b/test/functional/test-document-click.js @@ -27,6 +27,7 @@ describe('test-document-click onDocumentElementClick_', () => { let evt; let doc; let win; + let ampdoc; let history; let tgt; let elem; @@ -48,17 +49,22 @@ describe('test-document-click onDocumentElementClick_', () => { querySelectorSpy = sandbox.stub(); tgt = document.createElement('a'); tgt.href = 'https://www.google.com'; - doc = { - getElementById: getElementByIdSpy, - querySelector: querySelectorSpy, - defaultView: { - location: { - href: 'https://www.google.com/some-path?hello=world#link', - replace: replaceLocSpy, - }, + win = { + location: { + href: 'https://www.google.com/some-path?hello=world#link', + replace: replaceLocSpy, + }, + }; + ampdoc = { + win, + getRootNode: () => { + return { + getElementById: getElementByIdSpy, + querySelector: querySelectorSpy, + }; }, }; - win = doc.defaultView; + doc = {defaultView: win}; docElem = { ownerDocument: doc, }; @@ -87,7 +93,7 @@ describe('test-document-click onDocumentElementClick_', () => { it('should not do anything on path change', () => { tgt.href = 'https://www.google.com/some-other-path'; - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(0); expect(querySelectorSpy.callCount).to.equal(0); @@ -97,7 +103,7 @@ describe('test-document-click onDocumentElementClick_', () => { it('should not do anything on origin change', () => { tgt.href = 'https://maps.google.com/some-path#link'; - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(0); expect(querySelectorSpy.callCount).to.equal(0); @@ -107,7 +113,7 @@ describe('test-document-click onDocumentElementClick_', () => { it('should not do anything when there is no hash', () => { tgt.href = 'https://www.google.com/some-path'; - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(0); expect(querySelectorSpy.callCount).to.equal(0); @@ -117,7 +123,7 @@ describe('test-document-click onDocumentElementClick_', () => { it('should not do anything on a query change', () => { tgt.href = 'https://www.google.com/some-path?hello=foo#link'; - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(0); expect(querySelectorSpy.callCount).to.equal(0); @@ -136,7 +142,7 @@ describe('test-document-click onDocumentElementClick_', () => { it('should call getElementById on document', () => { getElementByIdSpy.returns(elem); expect(getElementByIdSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(1); expect(querySelectorSpy.callCount).to.equal(0); }); @@ -145,13 +151,13 @@ describe('test-document-click onDocumentElementClick_', () => { getElementByIdSpy.returns(null); querySelectorSpy.returns(null); expect(preventDefaultSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(preventDefaultSpy.callCount).to.equal(1); }); it('should not do anything if no anchor is found', () => { evt.target = document.createElement('span'); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(0); expect(querySelectorSpy.callCount).to.equal(0); }); @@ -160,7 +166,7 @@ describe('test-document-click onDocumentElementClick_', () => { 'found', () => { getElementByIdSpy.returns(null); expect(getElementByIdSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(1); expect(querySelectorSpy.callCount).to.equal(1); }); @@ -171,7 +177,7 @@ describe('test-document-click onDocumentElementClick_', () => { querySelectorSpy.returns(null); expect(getElementByIdSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(getElementByIdSpy.callCount).to.equal(1); expect(scrollIntoViewSpy.callCount).to.equal(0); expect(replaceLocSpy.callCount).to.equal(1); @@ -183,7 +189,7 @@ describe('test-document-click onDocumentElementClick_', () => { expect(replaceLocSpy.callCount).to.equal(0); expect(scrollIntoViewSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(scrollIntoViewSpy.callCount).to.equal(1); expect(replaceLocSpy.callCount).to.equal(1); expect(replaceLocSpy.args[0][0]).to.equal('#test'); @@ -195,7 +201,7 @@ describe('test-document-click onDocumentElementClick_', () => { expect(replaceLocSpy.callCount).to.equal(0); expect(scrollIntoViewSpy.callCount).to.equal(0); - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(scrollIntoViewSpy.callCount).to.equal(1); expect(replaceLocSpy.callCount).to.equal(1); expect(replaceLocSpy.args[0][0]).to.equal('#test'); @@ -212,7 +218,7 @@ describe('test-document-click onDocumentElementClick_', () => { viewport.scrollIntoView = () => { ops.push('scrollIntoView'); }; - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(ops).to.have.length(2); expect(ops[0]).to.equal('location.replace'); @@ -226,7 +232,7 @@ describe('test-document-click onDocumentElementClick_', () => { }); // Click -> push. - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(scrollIntoViewSpy.callCount).to.equal(0); expect(replaceLocSpy.callCount).to.equal(1); expect(replaceLocSpy.args[0][0]).to.equal('#test'); @@ -247,7 +253,7 @@ describe('test-document-click onDocumentElementClick_', () => { }); // Click -> push. - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(historyPushStub.callCount).to.equal(1); expect(replaceLocSpy.callCount).to.equal(1); expect(replaceLocSpy.args[0][0]).to.equal('#test'); @@ -272,7 +278,7 @@ describe('test-document-click onDocumentElementClick_', () => { }); it('should always open in _blank when embedded', () => { - onDocumentElementClick_(evt, viewport, history); + onDocumentElementClick_(evt, ampdoc, viewport, history); expect(win.open.called).to.be.true; expect(win.open.calledWith( 'ftp://example.com/a', '_blank')).to.be.true; @@ -293,7 +299,7 @@ describe('test-document-click onDocumentElementClick_', () => { }); it('should open link in _top on Safari iOS when embedded', () => { - onDocumentElementClick_(evt, viewport, history, true); + onDocumentElementClick_(evt, ampdoc, viewport, history, true); expect(win.open.called).to.be.true; expect(win.open.calledWith( 'whatsapp://send?text=hello', '_top')).to.be.true; @@ -302,19 +308,19 @@ describe('test-document-click onDocumentElementClick_', () => { it('should not do anything for mailto: protocol', () => { tgt.href = 'mailto:hello@example.com'; - onDocumentElementClick_(evt, viewport, history, true); + onDocumentElementClick_(evt, ampdoc, viewport, history, true); expect(win.open.called).to.be.false; expect(preventDefaultSpy.callCount).to.equal(0); }); it('should not do anything on other non-safari iOS', () => { - onDocumentElementClick_(evt, viewport, history, false); + onDocumentElementClick_(evt, ampdoc, viewport, history, false); expect(win.open.called).to.be.false; expect(preventDefaultSpy.callCount).to.equal(0); }); it('should not do anything on other platforms', () => { - onDocumentElementClick_(evt, viewport, history, false); + onDocumentElementClick_(evt, ampdoc, viewport, history, false); expect(win.top.location.href).to.equal('https://google.com'); expect(preventDefaultSpy.callCount).to.equal(0); });