From ba7ee252754e78e0ababeb4242c1cb3bf0096458 Mon Sep 17 00:00:00 2001 From: nainar Date: Thu, 22 Feb 2018 11:16:36 +1100 Subject: [PATCH] Force the 'data-embed-as' attribute to 'video' and make sure to show post text (#13454) * Force the 'data-embed-as' attribute to 'video' and make sure to show the post's text. * Strengthen regex and force data-embed-as attribute when not specifically set --- 3p/facebook.js | 13 ++++++++++- build-system/amp.extern.js | 1 + .../0.1/test/test-amp-facebook.js | 22 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/3p/facebook.js b/3p/facebook.js index 13560355c9e8..c9cf380e0bf0 100644 --- a/3p/facebook.js +++ b/3p/facebook.js @@ -49,10 +49,21 @@ function getPostContainer(global, data) { setStyle(c, 'text-align', 'center'); } const container = global.document.createElement('div'); - const embedAs = data.embedAs || 'post'; + let embedAs = data.embedAs || 'post'; user().assert(['post', 'video'].indexOf(embedAs) !== -1, 'Attribute data-embed-as for value is wrong, should be' + ' "post" or "video" was: %s', embedAs); + // If the user hasn't set the `data-embed-as` attribute and the provided href + // is a video, Force the `data-embed-as` attribute to 'video' and make sure + // to show the post's text. + if (data.href.match(/\/videos\/\d+\/?$/) && + !container.hasAttribute('data-embed-as')) { + embedAs = 'video'; + container.setAttribute('data-embed-as', 'video'); + // Since 'data-embed-as="video"' disables post text, setting the 'data-show-text' + // to 'true' enables the ability to see the text (changed from the default 'false') + container.setAttribute('data-show-text', 'true'); + } container.className = 'fb-' + embedAs; container.setAttribute('data-href', data.href); return container; diff --git a/build-system/amp.extern.js b/build-system/amp.extern.js index 497c467acb95..6520728df9ad 100644 --- a/build-system/amp.extern.js +++ b/build-system/amp.extern.js @@ -271,6 +271,7 @@ data.hideCta; data.smallHeader; data.adaptContainerWidth; data.showFacePile; +data.showText; // 3p code var twttr; diff --git a/extensions/amp-facebook/0.1/test/test-amp-facebook.js b/extensions/amp-facebook/0.1/test/test-amp-facebook.js index 77d0de55835c..980eda5886b1 100644 --- a/extensions/amp-facebook/0.1/test/test-amp-facebook.js +++ b/extensions/amp-facebook/0.1/test/test-amp-facebook.js @@ -125,6 +125,28 @@ describes.realWin('amp-facebook', { expect(fbVideo.getAttribute('data-href')).to.equal(fbVideoHref); }); + it('adds fb-video element with `data-embed-as` and `data-show-text` ' + + 'attributes set correctly', () => { + const div = doc.createElement('div'); + div.setAttribute('id', 'c'); + doc.body.appendChild(div); + win.context = { + tagName: 'AMP-FACEBOOK', + }; + + facebook(win, { + href: fbVideoHref, + width: 111, + height: 222, + }); + const fbVideo = doc.body.getElementsByClassName('fb-video')[0]; + expect(fbVideo).not.to.be.undefined; + expect(fbVideo.classList.contains('fb-video')).to.be.true; + expect(fbVideo.getAttribute('data-embed-as')).to.equal('video'); + expect(fbVideo.getAttribute('data-show-text')).to.equal('true'); + }); + + it('removes iframe after unlayoutCallback', () => { return getAmpFacebook(fbPostHref).then(ampFB => { const iframe = ampFB.querySelector('iframe');