From 19bc1f9f3029453642c9c349e50168ac59d96797 Mon Sep 17 00:00:00 2001 From: Prateek Bhatnagar Date: Mon, 19 Mar 2018 10:52:34 -0700 Subject: [PATCH] Subscriptions: dialog re-renderer (#14076) * dialog re-renderer * fixing comments * combining renderers * adding more attr to sanitizer test --- examples/amp-subscriptions.amp.html | 3 +- .../amp-subscriptions/0.1/dialog-renderer.js | 72 ----------- .../local-subscription-platform-renderer.js | 70 +++++++++-- .../0.1/test/test-dialog-renderer.js | 115 ------------------ .../test/test-local-subscription-rendering.js | 97 ++++++++++++++- src/sanitizer.js | 5 + test/functional/test-sanitizer.js | 13 ++ 7 files changed, 175 insertions(+), 200 deletions(-) delete mode 100644 extensions/amp-subscriptions/0.1/dialog-renderer.js delete mode 100644 extensions/amp-subscriptions/0.1/test/test-dialog-renderer.js diff --git a/examples/amp-subscriptions.amp.html b/examples/amp-subscriptions.amp.html index d9cdfd4de1fe..2b7e6c21a77e 100644 --- a/examples/amp-subscriptions.amp.html +++ b/examples/amp-subscriptions.amp.html @@ -334,7 +334,8 @@

Lorem Ipsum

Dialog for logged in users diff --git a/extensions/amp-subscriptions/0.1/dialog-renderer.js b/extensions/amp-subscriptions/0.1/dialog-renderer.js deleted file mode 100644 index d9ac857cab14..000000000000 --- a/extensions/amp-subscriptions/0.1/dialog-renderer.js +++ /dev/null @@ -1,72 +0,0 @@ -/** - * Copyright 2018 The AMP HTML Authors. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS-IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import {Services} from '../../../src/services'; -import {evaluateExpr} from './expr'; - - -export class DialogRenderer { - /** - * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc - * @param {!./dialog.Dialog} dialog - */ - constructor(ampdoc, dialog) { - /** @private @const {!../../../src/service/ampdoc-impl.AmpDoc} */ - this.ampdoc_ = ampdoc; - - /** @private @const {!./dialog.Dialog} */ - this.dialog_ = dialog; - - /** @private @const {!../../../src/service/template-impl.Templates} */ - this.templates_ = Services.templatesFor(ampdoc.win); - } - - /** - * @param {!JsonObject} authResponse - * @return {!Promise} - */ - render(authResponse) { - // Make sure the document is fully parsed. - return this.ampdoc_.whenReady().then(() => { - // Find the first matching dialog. - const candidates = this.ampdoc_.getRootNode() - .querySelectorAll('[subscriptions-dialog][subscriptions-display]'); - for (let i = 0; i < candidates.length; i++) { - const candidate = candidates[i]; - const expr = candidate.getAttribute('subscriptions-display'); - if (expr && evaluateExpr(expr, authResponse)) { - return candidate; - } - } - }).then(candidate => { - if (!candidate) { - return; - } - if (candidate.tagName == 'TEMPLATE') { - return this.templates_.renderTemplate(candidate, authResponse); - } - const clone = candidate.cloneNode(true); - clone.removeAttribute('subscriptions-dialog'); - clone.removeAttribute('subscriptions-display'); - return clone; - }).then(element => { - if (!element) { - return; - } - return this.dialog_.open(element, /* showCloseButton */ true); - }); - } -} diff --git a/extensions/amp-subscriptions/0.1/local-subscription-platform-renderer.js b/extensions/amp-subscriptions/0.1/local-subscription-platform-renderer.js index af36ac006b53..d1f70c63df71 100644 --- a/extensions/amp-subscriptions/0.1/local-subscription-platform-renderer.js +++ b/extensions/amp-subscriptions/0.1/local-subscription-platform-renderer.js @@ -14,8 +14,8 @@ * limitations under the License. */ -import {DialogRenderer} from './dialog-renderer'; import {Entitlement} from './entitlement'; +import {Services} from '../../../src/services'; import {evaluateExpr} from './expr'; /** @@ -35,8 +35,11 @@ export class LocalSubscriptionPlatformRenderer { /** @private @const */ this.rootNode_ = ampdoc.getRootNode(); - /** @private @const */ - this.dialogRenderer_ = new DialogRenderer(ampdoc, dialog); + /** @private @const {!./dialog.Dialog} */ + this.dialog_ = dialog; + + /** @private @const {!../../../src/service/template-impl.Templates} */ + this.templates_ = Services.templatesFor(ampdoc.win); } /** @@ -46,22 +49,74 @@ export class LocalSubscriptionPlatformRenderer { render(renderState) { return Promise.all([ this.renderActions_(renderState), - this.dialogRenderer_.render(/** @type {!JsonObject} */(renderState)), + this.renderDialog_(/** @type {!JsonObject} */(renderState)), ]); } /** - * * @param {!./amp-subscriptions.RenderState} renderState */ renderActions_(renderState) { + this.renderActionsInNode_(renderState, this.rootNode_); + } + + /** + * @param {!JsonObject} authResponse + * @return {!Promise} + */ + renderDialog_(authResponse) { + // Make sure the document is fully parsed. + return this.ampdoc_.whenReady().then(() => { + // Find the first matching dialog. + const candidates = this.ampdoc_.getRootNode() + .querySelectorAll('[subscriptions-dialog][subscriptions-display]'); + for (let i = 0; i < candidates.length; i++) { + const candidate = candidates[i]; + const expr = candidate.getAttribute('subscriptions-display'); + if (expr && evaluateExpr(expr, authResponse)) { + return candidate; + } + } + }).then(candidate => { + if (!candidate) { + return; + } + if (candidate.tagName == 'TEMPLATE') { + return this.templates_.renderTemplate(candidate, authResponse) + .then(element => { + const renderState = + /** @type {!./amp-subscriptions.RenderState} */(authResponse); + return this.renderActionsInNode_( + renderState, + element); + }); + } + const clone = candidate.cloneNode(true); + clone.removeAttribute('subscriptions-dialog'); + clone.removeAttribute('subscriptions-display'); + return clone; + }).then(element => { + if (!element) { + return; + } + return this.dialog_.open(element, /* showCloseButton */ true); + }); + } + + /** + * Renders actions inside a given node according to an authResponse + * @param {!./amp-subscriptions.RenderState} renderState + * @param {!Node} rootNode + * @return {!Promise} + * @private + */ + renderActionsInNode_(renderState, rootNode) { return this.ampdoc_.whenReady().then(() => { // Find the matching actions and sections and make them visible if evalutes to true. const querySelectors = '[subscriptions-action], [subscriptions-section="actions"],' + ' [subscriptions-actions]'; - const actionCandidates = - this.rootNode_.querySelectorAll(querySelectors); + const actionCandidates = rootNode.querySelectorAll(querySelectors); for (let i = 0; i < actionCandidates.length; i++) { const candidate = actionCandidates[i]; const expr = candidate.getAttribute('subscriptions-display'); @@ -70,6 +125,7 @@ export class LocalSubscriptionPlatformRenderer { candidate.setAttribute('i-amphtml-subs-display', ''); } } + return rootNode; }); } } diff --git a/extensions/amp-subscriptions/0.1/test/test-dialog-renderer.js b/extensions/amp-subscriptions/0.1/test/test-dialog-renderer.js deleted file mode 100644 index 1d83b133e2ea..000000000000 --- a/extensions/amp-subscriptions/0.1/test/test-dialog-renderer.js +++ /dev/null @@ -1,115 +0,0 @@ -/** - * Copyright 2018 The AMP HTML Authors. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS-IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as sinon from 'sinon'; -import {Dialog} from '../dialog'; -import {DialogRenderer} from '../dialog-renderer'; -import {Services} from '../../../../src/services'; -import {createElementWithAttributes} from '../../../../src/dom'; - - -describes.realWin('amp-subscriptions DialogRenderer', { - amp: {}, -}, env => { - let win, doc, ampdoc; - let templatesMock; - let dialogMock; - let renderer; - let dialog0, dialog1, dialog2, dialog3; - - beforeEach(() => { - win = env.win; - doc = win.document; - ampdoc = env.ampdoc; - templatesMock = sandbox.mock(Services.templatesFor(win)); - const dialog = new Dialog(ampdoc); - dialogMock = sandbox.mock(dialog); - renderer = new DialogRenderer(ampdoc, dialog); - dialog0 = createElementWithAttributes(doc, 'div', { - 'id': 'dialog0', - 'subscriptions-dialog': '', - 'subscriptions-display': '', - }); - dialog1 = createElementWithAttributes(doc, 'div', { - 'id': 'dialog1', - 'subscriptions-dialog': '', - 'subscriptions-display': 'value = "A"', - }); - dialog1.textContent = 'dialog1'; - dialog2 = createElementWithAttributes(doc, 'template', { - 'id': 'dialog2', - 'subscriptions-dialog': '', - 'subscriptions-display': 'value = "B"', - }); - dialog3 = createElementWithAttributes(doc, 'div', { - 'id': 'dialog3', - 'subscriptions-dialog': '', - 'subscriptions-display': 'value = "B"', - }); - doc.body.appendChild(dialog0); - doc.body.appendChild(dialog1); - doc.body.appendChild(dialog2); - doc.body.appendChild(dialog3); - }); - - afterEach(() => { - templatesMock.verify(); - dialogMock.verify(); - }); - - it('should render an element', () => { - templatesMock.expects('renderTemplate').never(); - let content; - dialogMock.expects('open') - .withExactArgs(sinon.match(arg => { - content = arg; - return true; - }), true) - .once(); - return renderer.render({value: 'A'}).then(() => { - expect(content.id).to.equal('dialog1'); - expect(content.textContent).to.equal('dialog1'); - expect(content).to.not.equal(dialog1); - expect(content).to.not.have.attribute('subscriptions-dialog'); - expect(content).to.not.have.attribute('subscriptions-display'); - }); - }); - - it('should render a template', () => { - const rendered = createElementWithAttributes(doc, 'div', {}); - const data = {value: 'B'}; - templatesMock.expects('renderTemplate') - .withExactArgs(dialog2, data) - .returns(Promise.resolve(rendered)) - .once(); - let content; - dialogMock.expects('open') - .withExactArgs(sinon.match(arg => { - content = arg; - return true; - }), true) - .once(); - return renderer.render(data).then(() => { - expect(content).to.equal(rendered); - }); - }); - - it('should ignore rendering if nothign found', () => { - templatesMock.expects('renderTemplate').never(); - dialogMock.expects('open').never(); - return renderer.render({value: 'C'}); - }); -}); diff --git a/extensions/amp-subscriptions/0.1/test/test-local-subscription-rendering.js b/extensions/amp-subscriptions/0.1/test/test-local-subscription-rendering.js index 0c9fa1b693c8..e2d44c7d64a2 100644 --- a/extensions/amp-subscriptions/0.1/test/test-local-subscription-rendering.js +++ b/extensions/amp-subscriptions/0.1/test/test-local-subscription-rendering.js @@ -13,22 +13,25 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - +import * as sinon from 'sinon'; +import {Dialog} from '../dialog'; import {Entitlement} from '../entitlement'; import {LocalSubscriptionPlatformRenderer} from '../local-subscription-platform-renderer'; +import {Services} from '../../../../src/services'; import {createElementWithAttributes} from '../../../../src/dom'; describes.realWin('local-subscriptions-rendering', {amp: true}, env => { - let win, doc; - let ampdoc; + let win, doc, ampdoc; let renderer; + let dialog; let entitlementsForService1; beforeEach(() => { win = env.win; doc = win.document; ampdoc = env.ampdoc; - renderer = new LocalSubscriptionPlatformRenderer(ampdoc); + dialog = new Dialog(ampdoc); + renderer = new LocalSubscriptionPlatformRenderer(ampdoc, dialog); const serviceIds = ['service1', 'service2']; const currentProduct = 'currentProductId'; const sampleEntitlement1 = @@ -41,7 +44,7 @@ describes.realWin('local-subscriptions-rendering', {amp: true}, env => { it('should call renderActions_ and renderDialog with ' + 'the entitlements provided', () => { const actionRenderStub = sandbox.stub(renderer, 'renderActions_'); - const dialogRenderStub = sandbox.stub(renderer.dialogRenderer_, 'render'); + const dialogRenderStub = sandbox.stub(renderer, 'renderDialog_'); renderer.render(entitlementsForService1); expect(actionRenderStub).to.be.calledWith(entitlementsForService1); expect(dialogRenderStub).to.be.calledWith(entitlementsForService1); @@ -96,4 +99,88 @@ describes.realWin('local-subscriptions-rendering', {amp: true}, env => { }); }); }); + + describe('dialog renderer', () => { + let templatesMock; + let dialogMock; + let dialog0, dialog1, dialog2, dialog3; + + beforeEach(() => { + templatesMock = sandbox.mock(Services.templatesFor(win)); + dialogMock = sandbox.mock(renderer.dialog_); + dialog0 = createElementWithAttributes(doc, 'div', { + 'id': 'dialog0', + 'subscriptions-dialog': '', + 'subscriptions-display': '', + }); + dialog1 = createElementWithAttributes(doc, 'div', { + 'id': 'dialog1', + 'subscriptions-dialog': '', + 'subscriptions-display': 'value = "A"', + }); + dialog1.textContent = 'dialog1'; + dialog2 = createElementWithAttributes(doc, 'template', { + 'id': 'dialog2', + 'subscriptions-dialog': '', + 'subscriptions-display': 'value = "B"', + }); + dialog3 = createElementWithAttributes(doc, 'div', { + 'id': 'dialog3', + 'subscriptions-dialog': '', + 'subscriptions-display': 'value = "B"', + }); + doc.body.appendChild(dialog0); + doc.body.appendChild(dialog1); + doc.body.appendChild(dialog2); + doc.body.appendChild(dialog3); + }); + + afterEach(() => { + templatesMock.verify(); + // dialogMock.verify(); + }); + + it('should render an element', () => { + templatesMock.expects('renderTemplate').never(); + let content; + dialogMock.expects('open') + .withExactArgs(sinon.match(arg => { + content = arg; + return true; + }), true) + .once(); + return renderer.renderDialog_({value: 'A'}).then(() => { + expect(content.id).to.equal('dialog1'); + expect(content.textContent).to.equal('dialog1'); + expect(content).to.not.equal(dialog1); + expect(content).to.not.have.attribute('subscriptions-dialog'); + expect(content).to.not.have.attribute('subscriptions-display'); + }); + }); + + it('should render a template', () => { + const rendered = createElementWithAttributes(doc, 'div', {}); + const data = {value: 'B'}; + templatesMock.expects('renderTemplate') + .withExactArgs(dialog2, data) + .returns(Promise.resolve(rendered)) + .once(); + let content; + dialogMock.expects('open') + .withExactArgs(sinon.match(arg => { + content = arg; + return true; + }), true) + .once(); + return renderer.renderDialog_(data).then(() => { + expect(content).to.equal(rendered); + }); + }); + + it('should ignore rendering if nothign found', () => { + templatesMock.expects('renderTemplate').never(); + dialogMock.expects('open').never(); + return renderer.render({value: 'C'}); + }); + }); }); diff --git a/src/sanitizer.js b/src/sanitizer.js index 23ceedda00ac..0be31b94d170 100644 --- a/src/sanitizer.js +++ b/src/sanitizer.js @@ -110,6 +110,11 @@ const WHITELISTED_ATTRS = [ /* Attributes added for amp-bind */ // TODO(kmh287): Add more whitelisted attributes for bind? 'text', + /* Attributes for amp-subscriptions */ + 'subscriptions-action', + 'subscriptions-actions', + 'subscriptions-section', + 'subscriptions-display', ]; /** @const {!Object>} */ diff --git a/test/functional/test-sanitizer.js b/test/functional/test-sanitizer.js index d0c43f157095..f7a205f1c606 100644 --- a/test/functional/test-sanitizer.js +++ b/test/functional/test-sanitizer.js @@ -227,6 +227,19 @@ describe('sanitizeHtml', () => { expect(sanitizeHtml('link')) .to.equal('link'); }); + + it('should allow amp-subscriptions attributes', () => { + expect(sanitizeHtml('
link
')) + .to.equal('
link
'); + expect(sanitizeHtml('
link
')) + .to.equal('
link
'); + expect(sanitizeHtml('
link
')) + .to.equal('
link
'); + expect(sanitizeHtml('
link
')) + .to.equal('
link
'); + expect(sanitizeHtml('
link
')) + .to.equal('
link
'); + }); });