Skip to content

Commit

Permalink
🚮 [amp-facebook] Remove support for the deprecated Embedded Comment…
Browse files Browse the repository at this point in the history 
…s. (#37498)

* [`amp-facebook`] Remove support for the deprecated Embedded Comments.

* [`bento-facebook`] Fix test failure for amp-facebook.

* [`bento-facebook`] Update tests to verify warning that `comment` is an
invalid embed-as type.

* [`amp-facebook`] Fix linting errors.
  • Loading branch information
@rbeckthomas
rbeckthomas committed Feb 1, 2022
1 parent 5ed4b81 commit b5fe886
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 86 deletions.
24 changes: 0 additions & 24 deletions 3p/facebook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ import {loadScript} from './3p';

/** @const @enum {string} */
export const FacebookEmbedType = {
// Embeds a single comment or reply to a comment on a post rendered by
// amp-facebook.
COMMENT: 'comment',
// Allows users to comment on the embedded content using their Facebook
// accounts. Correlates to amp-facebook-comments.
COMMENTS: 'comments',
Expand Down Expand Up @@ -92,25 +89,6 @@ function getVideoContainer(global, data) {
return container;
}

/**
* Create DOM element for the Facebook embedded content plugin for comments or
* comment replies.
* @see https://developers.facebook.com/docs/plugins/embedded-comments
* @param {!Window} global
* @param {!Object} data The element data
* @return {!Element} div
*/
function getCommentContainer(global, data) {
const c = global.document.getElementById('c');
const container = createContainer(global, 'comment-embed', data.href);
container.setAttribute(
'data-include-parent',
data.includeCommentParent || 'false'
);
container.setAttribute('data-width', c./*OK*/ offsetWidth);
return container;
}

/**
* Gets the default type to embed as, if not specified.
* @param {string} href
Expand Down Expand Up @@ -196,8 +174,6 @@ function getEmbedContainer(global, data, embedAs) {
return getLikeContainer(global, data);
case FacebookEmbedType.COMMENTS:
return getCommentsContainer(global, data);
case FacebookEmbedType.COMMENT:
return getCommentContainer(global, data);
case FacebookEmbedType.VIDEO:
return getVideoContainer(global, data);
default:
Expand Down
11 changes: 9 additions & 2 deletions extensions/amp-facebook/0.1/amp-facebook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,17 @@ class AmpFacebook extends AMP.BaseElement {
/** @override */
layoutCallback() {
const embedAs = this.element.getAttribute('data-embed-as');
if (embedAs === 'comment') {
this.user().warn(
'BENTO-LIGTHBOX',
'Embedded Comments have been deprecated: https://developers.facebook.com/docs/plugins/embedded-comments'
);
return;
}
userAssert(
!embedAs || ['post', 'video', 'comment'].indexOf(embedAs) !== -1,
!embedAs || ['post', 'video'].indexOf(embedAs) !== -1,
'Attribute data-embed-as for <amp-facebook> value is wrong, should be' +
' "post", "video" or "comment" but was: %s',
' "post" or "video" but was: %s',
embedAs
);
const iframe = getIframe(this.win, this.element, TYPE);
Expand Down
27 changes: 14 additions & 13 deletions extensions/amp-facebook/0.1/test/test-amp-facebook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {expect} from 'chai';

import {facebook} from '#3p/facebook';

import * as log from '#utils/log';

import {setDefaultBootstrapBaseUrlForTesting} from '../../../../src/3p-frame';
import {resetServiceForTesting} from '../../../../src/service-helpers';

Expand All @@ -20,6 +22,8 @@ describes.realWin(
const fbPostHref = 'https://www.facebook.com/zuck/posts/10102593740125791';
const fbVideoHref =
'https://www.facebook.com/zuck/videos/10102509264909801/';
const fbCommentHref =
'https://www.facebook.com/zuck/posts/10102735452532991?comment_id=1070233703036185';
let win, doc;

beforeEach(() => {
Expand Down Expand Up @@ -76,32 +80,29 @@ describes.realWin(
expect(context.attributes.embedAs).to.equal('video');
});

it('renders iframe in amp-facebook with comment', async () => {
const ampFB = await getAmpFacebook(fbVideoHref, 'comment');
const iframe = ampFB.firstChild;
expect(iframe).to.not.be.null;
expect(iframe.tagName).to.equal('IFRAME');
expect(iframe.className).to.match(/i-amphtml-fill-content/);

const context = JSON.parse(iframe.getAttribute('name'));
expect(context.attributes.embedAs).to.equal('comment');
it('warns unsupported data-embed-as value: comment', async () => {
const warn = env.sandbox.spy();
env.sandbox.stub(log, 'user').returns({warn});
expect(warn).not.to.be.called;
await getAmpFacebook(fbCommentHref, 'comment');
expect(warn).to.be.calledOnce;
});

it('rejects other supported and unsupported data-embed-as types', async () => {
expectAsyncConsoleError(/.*/);
await expect(getAmpFacebook(fbVideoHref, 'comments')).to.be.rejectedWith(
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post", "video" or "comment" but was: comments/
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post" or "video" but was: comments/
);
await expect(getAmpFacebook(fbVideoHref, 'like')).to.be.rejectedWith(
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post", "video" or "comment" but was: like/
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post" or "video" but was: like/
);
await expect(getAmpFacebook(fbVideoHref, 'page')).to.be.rejectedWith(
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post", "video" or "comment" but was: page/
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post" or "video" but was: page/
);
await expect(
getAmpFacebook(fbVideoHref, 'unsupported')
).to.be.rejectedWith(
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post", "video" or "comment" but was: unsupported/
/Attribute data-embed-as for <amp-facebook> value is wrong, should be "post" or "video" but was: unsupported/
);
});

Expand Down
10 changes: 6 additions & 4 deletions extensions/amp-facebook/1.0/base-element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ function parseEmbed(element) {
const embedAs = element.getAttribute('data-embed-as');
userAssert(
!embedAs ||
['post', 'video', 'comment', 'comments', 'like', 'page'].indexOf(
embedAs
) !== -1,
['post', 'video', 'comments', 'like', 'page'].indexOf(embedAs) !== -1,
'Attribute data-embed-as for <amp-facebook> value is wrong, should be' +
' "post", "video", "comment", "comments", "like", or "page", but was: %s',
' "post", "video", "comments", "like", or "page", but was: %s',
embedAs
);
userAssert(
embedAs !== 'comment',
'Embedded Comments have been deprecated: https://developers.facebook.com/docs/plugins/embedded-comments'
);
return embedAs;
}

Expand Down
51 changes: 8 additions & 43 deletions extensions/amp-facebook/1.0/test/test-amp-facebook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ describes.realWin(
const fbPostHref =
'https://www.facebook.com/NASA/photos/a.67899501771/10159193669016772/';
const fbVideoHref = 'https://www.facebook.com/NASA/videos/846648316199961/';
const fbCommentHref =
'https://www.facebook.com/NASA/photos/a.67899501771/10159193669016772/?comment_id=10159193676606772';
const fbCommentsHref =
'http://www.directlyrics.com/adele-25-complete-album-lyrics-news.html';

beforeEach(() => {
win = env.win;
Expand Down Expand Up @@ -204,55 +204,20 @@ describes.realWin(
expect(fbVideo.classList.contains('fb-post')).to.be.true;
});

it('adds fb-comment element correctly', () => {
it('adds fb-comments element correctly', () => {
const div = doc.createElement('div');
div.setAttribute('id', 'c');
doc.body.appendChild(div);

facebook(win, {
href: fbCommentHref,
embedAs: 'comment',
href: fbCommentsHref,
embedAs: 'comments',
width: 111,
height: 222,
});
const fbComment = doc.body.getElementsByClassName('fb-comment-embed')[0];
expect(fbComment).not.to.be.undefined;
expect(fbComment.getAttribute('data-href')).to.equal(fbCommentHref);
});

it('adds fb-comment element with include-parent', () => {
const div = doc.createElement('div');
div.setAttribute('id', 'c');
doc.body.appendChild(div);

facebook(win, {
href: fbCommentHref,
embedAs: 'comment',
includeCommentParent: true,
width: 111,
height: 222,
});
const fbComment = doc.body.getElementsByClassName('fb-comment-embed')[0];
expect(fbComment).not.to.be.undefined;
expect(fbComment.getAttribute('data-href')).to.equal(fbCommentHref);
expect(fbComment.getAttribute('data-include-parent')).to.equal('true');
});

it('adds fb-comment element with default include-parent', () => {
const div = doc.createElement('div');
div.setAttribute('id', 'c');
doc.body.appendChild(div);

facebook(win, {
href: fbCommentHref,
embedAs: 'comment',
width: 111,
height: 222,
});
const fbComment = doc.body.getElementsByClassName('fb-comment-embed')[0];
expect(fbComment).not.to.be.undefined;
expect(fbComment.getAttribute('data-href')).to.equal(fbCommentHref);
expect(fbComment.getAttribute('data-include-parent')).to.equal('false');
const fbComments = doc.body.getElementsByClassName('fb-comments')[0];
expect(fbComments).not.to.be.undefined;
expect(fbComments.getAttribute('data-href')).to.equal(fbCommentsHref);
});

it("container's height is changed", async () => {
Expand Down

0 comments on commit b5fe886

Please sign in to comment.