Skip to content

Commit

Permalink
Install global submit listener only when amp-form extension is regist…
Browse files Browse the repository at this point in the history
…ered (ampproject#17246)

* install global submit listener only when amp-form extension is registered

* fix tests

* address comments

* fixes

* wait for body before checking for script in head

* fix test

* add comment

* address comments

* Fix form recursive submit test.

* Remove comment.
  • Loading branch information
alabiaga authored and kevinkassimo committed Aug 9, 2018
1 parent a80b161 commit 6eb4f81
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 7 deletions.
17 changes: 13 additions & 4 deletions extensions/amp-form/0.1/test/integration/test-integration-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {AmpEvents} from '../../../../../src/amp-events';
import {AmpForm, AmpFormService} from '../../amp-form';
import {AmpMustache} from '../../../../amp-mustache/0.1/amp-mustache';
import {installGlobalSubmitListenerForDoc} from '../../../../../src/document-submit';
import {listenOncePromise} from '../../../../../src/event-helper';
import {poll} from '../../../../../testing/iframe';
import {registerExtendedTemplate} from
Expand Down Expand Up @@ -48,11 +49,20 @@ describes.realWin('AmpForm Integration', {
beforeEach(() => {
sandbox = env.sandbox;
doc = env.win.document;
const scriptElement = document.createElement('script');
scriptElement.setAttribute('custom-template', 'amp-mustache');
doc.body.appendChild(scriptElement);

const mustache = document.createElement('script');
mustache.setAttribute('custom-template', 'amp-mustache');
doc.body.appendChild(mustache);
registerExtendedTemplate(env.win, 'amp-mustache', AmpMustache);

const form = document.createElement('script');
form.setAttribute('custom-element', 'amp-form');
doc.head.appendChild(form);

new AmpFormService(env.ampdoc);

// Wait for submit listener to be installed before starting tests.
return installGlobalSubmitListenerForDoc(env.ampdoc);
});

function getForm(config) {
Expand Down Expand Up @@ -139,7 +149,6 @@ describes.realWin('AmpForm Integration', {
// The second time in response to the `submit` event being triggered
// and sameform.submit being invoked.
expect(ampForm.handleSubmitAction_).to.have.been.calledTwice;

// However, only the first invocation should be handled completely.
// and any subsequent calls should be stopped early-on.
expect(ampForm.handleXhrSubmit_).to.have.been.calledOnce;
Expand Down
12 changes: 11 additions & 1 deletion src/document-submit.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,22 @@ import {
} from './url';
import {Services} from './services';
import {dev, user} from './log';
import {isExtensionScriptInNode} from './element-service';

/**
* @param {!./service/ampdoc-impl.AmpDoc} ampdoc
* @return {!Promise}
*/
export function installGlobalSubmitListenerForDoc(ampdoc) {
ampdoc.getRootNode().addEventListener('submit', onDocumentFormSubmit_, true);
// Register global submit event listener only if the amp-form
// extension is used. Allowing the usage of native forms, otherwise.
return isExtensionScriptInNode(ampdoc, 'amp-form')
.then(ampFormInstalled => {
if (ampFormInstalled) {
ampdoc.getRootNode().addEventListener(
'submit', onDocumentFormSubmit_, true);
}
});
}


Expand Down
28 changes: 27 additions & 1 deletion src/element-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,32 @@ export function extensionScriptsInNode(head) {
return scripts;
}

/**
* Waits for body to be present then verifies that an extension script is
* present in head for installation.
* @param {!./service/ampdoc-impl.AmpDoc} ampdoc
* @param {string} extensionId
* @return {!Promise<boolean>}
*/
export function isExtensionScriptInNode(ampdoc, extensionId) {
return ampdoc.whenBodyAvailable()
.then(() => {
return extensionScriptInNode(
ampdoc.getHeadNode(), extensionId);
});
}

/**
* Verifies that an extension script is present in head for
* installation.
* @param {HTMLHeadElement|Element|ShadowRoot} head
* @param {string} extensionId
* @private
*/
function extensionScriptInNode(head, extensionId) {
return extensionScriptsInNode(head).includes(extensionId);
}

/**
* Waits for an extension if its script is present
* @param {!Window} win
Expand All @@ -222,7 +248,7 @@ function waitForExtensionIfPresent(win, extension, head) {

// TODO(jpettitt) investigate registerExtension to short circuit
// the dom call in extensionScriptsInNode()
if (!extensionScriptsInNode(head).includes(extension)) {
if (!extensionScriptInNode(head, extension)) {
return Promise.resolve();
}

Expand Down
45 changes: 44 additions & 1 deletion test/functional/test-document-submit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {Services} from '../../src/services';
import {onDocumentFormSubmit_} from '../../src/document-submit';
import {installGlobalSubmitListenerForDoc, onDocumentFormSubmit_} from '../../src/document-submit';

describe('test-document-submit onDocumentFormSubmit_', () => {
let sandbox;
Expand Down Expand Up @@ -46,6 +46,49 @@ describe('test-document-submit onDocumentFormSubmit_', () => {
sandbox.restore();
});

describe('installGlobalSubmitListenerForDoc', () => {
let ampdoc;
let headNode;
let rootNode;
beforeEach(() => {
headNode = document.createElement('head');
rootNode = document.createElement('html');
ampdoc = {
getHeadNode: () => headNode,
getRootNode: () => rootNode,
whenBodyAvailable: () => Promise.resolve({}),
};
});

/**
* @param {string} extension
*/
const createScript = extension => {
const script = document.createElement('script');
script.setAttribute('src',
'https://cdn.ampproject.org/v0/' + extension + '-0.1.js');
script.setAttribute('custom-element', extension);
return script;
};

it('should not register submit listener if amp-form is not registered.',
() => {
ampdoc.getHeadNode().appendChild(createScript('amp-list'));
sandbox.spy(rootNode, 'addEventListener');
return installGlobalSubmitListenerForDoc(ampdoc).then(() => {
expect(rootNode.addEventListener).not.to.have.been.called;
});
});

it('should register submit listener if amp-form extension is registered.',
() => {
ampdoc.getHeadNode().appendChild(createScript('amp-form'));
sandbox.spy(rootNode, 'addEventListener');
return installGlobalSubmitListenerForDoc(ampdoc).then(() => {
expect(rootNode.addEventListener).called;
});
});
});

it('should check target and action attributes', () => {
tgt.removeAttribute('action');
Expand Down
12 changes: 12 additions & 0 deletions test/functional/test-element-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
getElementServiceIfAvailable,
getElementServiceIfAvailableForDoc,
getElementServiceIfAvailableForDocInEmbedScope,
isExtensionScriptInNode,
} from '../../src/element-service';
import {
installServiceInEmbedScope,
Expand Down Expand Up @@ -330,6 +331,17 @@ describes.realWin('in single ampdoc', {
expect(service).to.deep.equal({str: 'fake1'});
});
});

it('isExtensionScriptInNode', () => {
const extension = document.createElement('script');
extension.setAttribute('custom-element', 'amp-form');
extension.setAttribute('src', 'https://cdn.ampproject.org/v0/amp-form-0.1.js');
ampdoc.getHeadNode().appendChild(extension);
return isExtensionScriptInNode(ampdoc, 'amp-form')
.then(ampFormInstalled => {
expect(ampFormInstalled).to.equal(true);
});
});
});
});

Expand Down

0 comments on commit 6eb4f81

Please sign in to comment.