From 22b70b04b34337ceac66504c5947aadd8cb1523b Mon Sep 17 00:00:00 2001 From: Matt Mower Date: Fri, 3 Apr 2020 10:42:43 -0700 Subject: [PATCH] amp-apester: Support keyword lookup in AmpDocShadow (#27345) * amp-apester: Support keyword lookup in AmpDocShadow Query meta[name="keywords"] using method supported by AmpDocShadow as well as other AmpDoc subtypes. * extratctTitle --> extractTitle * Add test note about clearing meta_ --- .../0.1/amp-apester-media.js | 2 +- .../0.1/test/test-amp-apester-media-utils.js | 30 ++++++++++++------- extensions/amp-apester-media/0.1/utils.js | 30 +++++++++---------- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/extensions/amp-apester-media/0.1/amp-apester-media.js b/extensions/amp-apester-media/0.1/amp-apester-media.js index ede4ca541669..e7e3ca3e22e4 100644 --- a/extensions/amp-apester-media/0.1/amp-apester-media.js +++ b/extensions/amp-apester-media/0.1/amp-apester-media.js @@ -152,7 +152,7 @@ class AmpApesterMedia extends AMP.BaseElement { 'data-apester-channel-id' ), renderer: true, - tags: extractTags(this.getAmpDoc().getRootNode(), this.element), + tags: extractTags(this.getAmpDoc(), this.element), }; } diff --git a/extensions/amp-apester-media/0.1/test/test-amp-apester-media-utils.js b/extensions/amp-apester-media/0.1/test/test-amp-apester-media-utils.js index c4c96ad0e398..e09c99a4e7de 100644 --- a/extensions/amp-apester-media/0.1/test/test-amp-apester-media-utils.js +++ b/extensions/amp-apester-media/0.1/test/test-amp-apester-media-utils.js @@ -14,11 +14,12 @@ * limitations under the License. */ +import {Services} from '../../../../src/services'; import { extractArticleTags, extractElementTags, extractTags, - extratctTitle, + extractTitle, } from '../utils'; describes.realWin('amp-apester-media-utils', {}, (unused) => { @@ -26,6 +27,10 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { document.body.textContent = ''; document.head.textContent = ''; }); + afterEach(() => { + // Clear cached AmpDoc meta since document is reused in each test + Services.ampdoc(document).meta_ = null; + }); it('Extract element tags as empty array when there is no element', () => { const expected = []; @@ -51,7 +56,7 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { it('Extract title works well with no dl json', () => { const expected = []; - const tags = extratctTitle(document); + const tags = extractTitle(document); expect(tags).to.deep.equal(expected); }); @@ -76,7 +81,7 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { }`; element.text = jsonLd; document.body.appendChild(element); - const tags = extratctTitle(document); + const tags = extractTitle(document); expect(tags).to.deep.equal(expected); }); @@ -101,7 +106,7 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { }`; element.text = jsonLd; document.body.appendChild(element); - const tags = extratctTitle(document); + const tags = extractTitle(document); expect(tags).to.deep.equal(expected); }); @@ -129,7 +134,7 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { document.body.appendChild(element); } - const tags = extratctTitle(document); + const tags = extractTitle(document); expect(tags).to.deep.equal(expected); }); @@ -139,7 +144,8 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { meta.setAttribute('name', 'keywords'); meta.setAttribute('content', ''); document.head.appendChild(meta); - const tags = extractArticleTags(document); + const ampdoc = Services.ampdoc(document); + const tags = extractArticleTags(ampdoc); expect(tags).to.deep.equal(expected); }); @@ -149,7 +155,8 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { meta.setAttribute('name', 'keywords'); meta.setAttribute('content', ''); document.head.appendChild(meta); - const tags = extractArticleTags(document); + const ampdoc = Services.ampdoc(document); + const tags = extractArticleTags(ampdoc); expect(tags).to.deep.equal(expected); }); @@ -159,7 +166,8 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { meta.setAttribute('name', 'keywords'); meta.setAttribute('content', 'this is, the, tag'); document.head.appendChild(meta); - const tags = extractArticleTags(document); + const ampdoc = Services.ampdoc(document); + const tags = extractArticleTags(ampdoc); expect(tags).to.deep.equal(expected); }); @@ -201,7 +209,8 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { scriptElement.text = jsonLd; document.body.appendChild(scriptElement); - const tags = extractTags(document, element); + const ampdoc = Services.ampdoc(document); + const tags = extractTags(ampdoc, element); expect(tags).to.deep.equal(expected); }); @@ -233,7 +242,8 @@ describes.realWin('amp-apester-media-utils', {}, (unused) => { scriptElement.text = jsonLd; document.body.appendChild(scriptElement); - const tags = extractTags(document, element); + const ampdoc = Services.ampdoc(document); + const tags = extractTags(ampdoc, element); expect(tags).to.deep.equal(expected); }); }); diff --git a/extensions/amp-apester-media/0.1/utils.js b/extensions/amp-apester-media/0.1/utils.js index 49707efd44a0..94dbc62cdab7 100644 --- a/extensions/amp-apester-media/0.1/utils.js +++ b/extensions/amp-apester-media/0.1/utils.js @@ -88,10 +88,10 @@ export function extractElementTags(element) { /** * Extract article's first 5 words from the title and use them for tags. - * @param {!Node} root + * @param {!Document|!ShadowRoot} root * @return {!Array} */ -export function extratctTitle(root) { +export function extractTitle(root) { const scriptTags = toArray( root.querySelectorAll('script[type="application/ld+json"]') ); @@ -117,31 +117,29 @@ export function extratctTitle(root) { } /** * Extracts article meta keywords - * @param {*} root - * @return {*} TODO(#23582): Specify return type + * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc + * @return {Array} */ -export function extractArticleTags(root) { - const metaKeywords = root.querySelector('meta[name="keywords"]') || { - content: '', - }; - return metaKeywords.content - .trim() +export function extractArticleTags(ampdoc) { + return (ampdoc.getMetaByName('keywords') || '') .split(',') - .filter((e) => e) - .map((e) => e.trim()); + .map((e) => e.trim()) + .filter((e) => e); } /** * Extracts tags from a given element and document. - * @param {!Node} root + * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!Node} element * @return {Array} */ -export function extractTags(root, element) { +export function extractTags(ampdoc, element) { const extractedTags = extractElementTags(element) || []; - const articleMetaTags = extractArticleTags(root); + const articleMetaTags = extractArticleTags(ampdoc); const concatedTags = extractedTags.concat( - articleMetaTags.length ? articleMetaTags : extratctTitle(root) || [] + articleMetaTags.length + ? articleMetaTags + : extractTitle(ampdoc.getRootNode()) || [] ); const loweredCase = concatedTags.map((tag) => tag.toLowerCase().trim()); const noDuplication = loweredCase.filter(