Skip to content

Commit ff8e61c

Browse files
westonruterwestonruter
authored
Merge pull request #4384 from ampproject/fix/4358-wrapped-facebook-component
Remove registration of Facebook embed handler
2 parents 1ccc0fa + fe545a5 commit ff8e61c

File tree

3 files changed

+121
-132
lines changed

3 files changed

+121
-132
lines changed

‎Gruntfile.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ module.exports = function( grunt ) {
3939
'vendor/*/*/*.yml',
4040
'vendor/*/*/.*.yml',
4141
'vendor/*/*/tests',
42+
'vendor/ampproject/common/phpstan.neon.dist',
4243
'vendor/ampproject/optimizer/bin',
44+
'vendor/ampproject/optimizer/phpstan.neon.dist',
4345
'vendor/bin',
4446
];
4547

‎includes/embeds/class-amp-facebook-embed.php‎

Lines changed: 21 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,6 @@
1111
* Class AMP_Facebook_Embed_Handler
1212
*/
1313
class AMP_Facebook_Embed_Handler extends AMP_Base_Embed_Handler {
14-
const URL_PATTERN = '#https?://(www\.)?facebook\.com/.*#i';
15-
16-
/**
17-
* Default width.
18-
*
19-
* @var int
20-
*/
21-
protected $DEFAULT_WIDTH = 600;
22-
2314
/**
2415
* Default height.
2516
*
@@ -45,57 +36,14 @@ class AMP_Facebook_Embed_Handler extends AMP_Base_Embed_Handler {
4536
* Registers embed.
4637
*/
4738
public function register_embed() {
48-
wp_embed_register_handler( $this->amp_tag, self::URL_PATTERN, [ $this, 'oembed' ], -1 );
39+
// Not implemented.
4940
}
5041

5142
/**
5243
* Unregisters embed.
5344
*/
5445
public function unregister_embed() {
55-
wp_embed_unregister_handler( $this->amp_tag, -1 );
56-
}
57-
58-
/**
59-
* WordPress OEmbed rendering callback.
60-
*
61-
* @param array $matches URL pattern matches.
62-
* @param array $attr Matched attributes.
63-
* @param string $url Matched URL.
64-
* @return string HTML markup for rendered embed.
65-
*/
66-
public function oembed( $matches, $attr, $url ) {
67-
return $this->render( [ 'url' => $url ] );
68-
}
69-
70-
/**
71-
* Gets the rendered embed markup.
72-
*
73-
* @param array $args Embed rendering arguments.
74-
* @return string HTML markup for rendered embed.
75-
*/
76-
public function render( $args ) {
77-
$args = wp_parse_args(
78-
$args,
79-
[
80-
'url' => false,
81-
]
82-
);
83-
84-
if ( empty( $args['url'] ) ) {
85-
return '';
86-
}
87-
88-
$this->did_convert_elements = true;
89-
90-
return AMP_HTML_Utils::build_tag(
91-
$this->amp_tag,
92-
[
93-
'data-href' => $args['url'],
94-
'layout' => 'responsive',
95-
'width' => $this->args['width'],
96-
'height' => $this->args['height'],
97-
]
98-
);
46+
// Not implemented.
9947
}
10048

10149
/**
@@ -131,14 +79,27 @@ public function sanitize_raw_embeds( Document $dom ) {
13179
*/
13280
$fb_root = $dom->getElementById( 'fb-root' );
13381
if ( $fb_root ) {
134-
$scripts = [];
135-
foreach ( $dom->xpath->query( '//script[ starts-with( @src, "https://connect.facebook.net" ) and contains( @src, "sdk.js" ) ]' ) as $script ) {
136-
$scripts[] = $script;
82+
/**
83+
* Script.
84+
*
85+
* @var DOMElement $script_query
86+
*/
87+
$script_query = $dom->xpath->query( '//script[ starts-with( @src, "https://connect.facebook.net" ) and contains( @src, "sdk.js" ) ]' );
88+
foreach ( $script_query as $script ) {
89+
$parent_node = $script->parentNode;
90+
$parent_node->removeChild( $script );
91+
92+
// Remove parent node if it is an empty <p> tag.
93+
if ( 'p' === $parent_node->nodeName && null === $parent_node->firstChild ) {
94+
$parent_node->parentNode->removeChild( $parent_node );
95+
}
13796
}
138-
foreach ( $scripts as $script ) {
139-
$script->parentNode->removeChild( $script );
97+
98+
// Remove other instances of <div id="fb-root">.
99+
$fb_root_query = $dom->xpath->query( '//div[ @id = "fb-root" ]' );
100+
foreach ( $fb_root_query as $fb_root ) {
101+
$fb_root->parentNode->removeChild( $fb_root );
140102
}
141-
$fb_root->parentNode->removeChild( $fb_root );
142103
}
143104
}
144105

‎tests/php/test-amp-facebook-embed.php‎

Lines changed: 98 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -12,60 +12,6 @@
1212
*/
1313
class AMP_Facebook_Embed_Test extends WP_UnitTestCase {
1414

15-
/**
16-
* Data provider for test__conversion.
17-
*
18-
* @return array Data.
19-
*/
20-
public function get_conversion_data() {
21-
return [
22-
'no_embed' => [
23-
'<p>Hello world.</p>',
24-
'<p>Hello world.</p>' . PHP_EOL,
25-
],
26-
'simple_url_https' => [
27-
'https://www.facebook.com/zuck/posts/10102593740125791' . PHP_EOL,
28-
'<p><amp-facebook data-href="https://www.facebook.com/zuck/posts/10102593740125791" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
29-
],
30-
'simple_url_http' => [
31-
'http://www.facebook.com/zuck/posts/10102593740125791' . PHP_EOL,
32-
'<p><amp-facebook data-href="http://www.facebook.com/zuck/posts/10102593740125791" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
33-
],
34-
'no_dubdubdub' => [
35-
'https://facebook.com/zuck/posts/10102593740125791' . PHP_EOL,
36-
'<p><amp-facebook data-href="https://facebook.com/zuck/posts/10102593740125791" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
37-
],
38-
'notes_url' => [
39-
'https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/' . PHP_EOL,
40-
'<p><amp-facebook data-href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
41-
],
42-
'photo_url' => [
43-
'https://www.facebook.com/photo.php?fbid=10102533316889441&set=a.529237706231.2034669.4&type=3&theater' . PHP_EOL,
44-
'<p><amp-facebook data-href="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231.2034669.4&amp;type=3&amp;theater" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
45-
],
46-
'notes_url2' => [
47-
'https://www.facebook.com/zuck/videos/10102509264909801/' . PHP_EOL,
48-
'<p><amp-facebook data-href="https://www.facebook.com/zuck/videos/10102509264909801/" layout="responsive" width="600" height="400"></amp-facebook></p>' . PHP_EOL,
49-
],
50-
51-
];
52-
}
53-
54-
/**
55-
* Test conversion.
56-
*
57-
* @dataProvider get_conversion_data
58-
* @param string $source Source.
59-
* @param string $expected Expected.
60-
*/
61-
public function test__conversion( $source, $expected ) {
62-
$embed = new AMP_Facebook_Embed_Handler();
63-
$embed->register_embed();
64-
$filtered_content = apply_filters( 'the_content', $source );
65-
66-
$this->assertEqualMarkup( $expected, $filtered_content );
67-
}
68-
6915
/**
7016
* Get scripts data.
7117
*
@@ -94,9 +40,12 @@ public function get_scripts_data() {
9440
public function test__get_scripts( $source, $expected ) {
9541
$embed = new AMP_Facebook_Embed_Handler();
9642
$embed->register_embed();
97-
$source = apply_filters( 'the_content', $source );
9843

99-
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( AMP_DOM_Utils::get_dom_from_content( $source ) );
44+
$filtered_content = apply_filters( 'the_content', $source );
45+
$dom = AMP_DOM_Utils::get_dom_from_content( $filtered_content );
46+
$embed->sanitize_raw_embeds( $dom );
47+
48+
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
10049
$whitelist_sanitizer->sanitize();
10150

10251
$scripts = array_merge(
@@ -114,21 +63,79 @@ public function test__get_scripts( $source, $expected ) {
11463
*/
11564
public function get_raw_embed_dataset() {
11665
return [
117-
'no_embed_blockquote' => [
66+
'no_embed_blockquote' => [
11867
'<p>Hello world.</p>',
11968
'<p>Hello world.</p>',
12069
],
121-
'div_without_instagram' => [
70+
71+
'div_without_facebook_embed' => [
12272
'<div>lorem ipsum</div>',
12373
'<div>lorem ipsum</div>',
12474
],
12575

126-
'post_embed' => [
76+
'simple_url_https' => [
77+
'https://www.facebook.com/zuck/posts/10102593740125791' . PHP_EOL,
78+
'
79+
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/zuck/posts/10102593740125791" data-embed-as="post" layout="responsive">
80+
<blockquote cite="https://www.facebook.com/zuck/posts/10102593740125791" class="fb-xfbml-parse-ignore" fallback="">
81+
<p>February 4 is Facebook’s 12th birthday!</p>
82+
<p>Our anniversary has a lot of meaning to me as an opportunity to reflect on how…</p>
83+
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on <a href="https://www.facebook.com/zuck/posts/10102593740125791">Tuesday, January 12, 2016</a></p>
84+
</blockquote>
85+
</amp-facebook>
86+
' . PHP_EOL,
87+
],
88+
89+
'notes_url' => [
90+
'https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/' . PHP_EOL,
91+
'
92+
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" data-embed-as="post" layout="responsive">
93+
<blockquote cite="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" class="fb-xfbml-parse-ignore" fallback="">
94+
<p>Posted by <a href="https://www.facebook.com/Engineering/">Facebook Engineering</a> on <a href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/">Saturday, December 8, 2012</a></p>
95+
</blockquote>
96+
</amp-facebook>
97+
' . PHP_EOL,
98+
],
99+
100+
'photo_url' => [
101+
'https://www.facebook.com/photo.php?fbid=10102533316889441&set=a.529237706231.2034669.4&type=3&theater' . PHP_EOL,
102+
'
103+
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231.2034669.4&amp;type=3&amp;theater" data-embed-as="post" layout="responsive">
104+
<blockquote cite="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231&amp;type=3" class="fb-xfbml-parse-ignore" fallback="">
105+
<p>Meanwhile, Beast turned to the dark side.</p>
106+
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on <a href="https://www.facebook.com/photo.php?fbid=10102533316889441&amp;set=a.529237706231&amp;type=3">Friday, December 18, 2015</a></p>
107+
</blockquote>
108+
</amp-facebook>
109+
' . PHP_EOL,
110+
],
111+
112+
'video_url' => [
113+
'https://www.facebook.com/zuck/videos/10102509264909801/' . PHP_EOL,
114+
'
115+
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/zuck/videos/10102509264909801/" data-embed-as="video" layout="responsive">
116+
<blockquote cite="https://www.facebook.com/zuck/videos/10102509264909801/" class="fb-xfbml-parse-ignore" fallback="">
117+
<p><a href="https://www.facebook.com/zuck/videos/10102509264909801/"></a></p>
118+
<p>I want to share a few more thoughts on the Chan Zuckerberg Initiative before I just start posting photos of me and Max for a while 🙂</p>
119+
<p>I hope one idea comes through: that we as a society should make investments now to ensure this world is much better for the next generation.</p>
120+
<p>I don\'t think we do enough of this right now. </p>
121+
<p>Sure, there are many areas where investment now will solve problems for today and also improve the world for the future. We do muster the will to solve some of those.</p>
122+
<p>But for the problems that will truly take decades of investment before we see any major return, we are dramatically underinvested.</p>
123+
<p>One example is basic science research to cure disease. Another is developing clean energy to protect the world for the future. Another is the slow and steady improvement to modernize schools. Others are systematic issues around poverty and justice. There is a long list of these opportunities.</p>
124+
<p>The role of philanthropy is to invest in important areas that companies and governments aren\'t funding — either because they may not be profitable for companies or because they are too long term for people to want to invest now.</p>
125+
<p>In the case of disease, basic research often needs to be funded before biotech or pharma companies can create drugs to help people. If we invest more in science, we can make faster progress towards curing disease.</p>
126+
<p>Our investment in the Chan Zuckerberg Initiative is small compared to what the world can invest in solving these great challenges. My hope is that our work inspires more people to invest in these longer term issues. If we can do that, then we can all really make a difference together.</p>
127+
<p>Posted by <a href="https://www.facebook.com/zuck">Mark Zuckerberg</a> on Friday, December 4, 2015</p>
128+
</blockquote>
129+
</amp-facebook>
130+
' . PHP_EOL,
131+
],
132+
133+
'post_embed' => [
127134
'<div class="fb-post" data-href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/"></div>',
128135
'<amp-facebook width="600" height="400" data-href="https://www.facebook.com/notes/facebook-engineering/under-the-hood-the-javascript-sdk-truly-asynchronous-loading/10151176218703920/" data-embed-as="post" layout="responsive"></amp-facebook>',
129136
],
130137

131-
'post_with_fallbacks' => [
138+
'post_with_fallbacks' => [
132139
'
133140
<div class="fb-post" data-href="https://www.facebook.com/20531316728/posts/10154009990506729/" data-width="500" data-show-text="true">
134141
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore">
@@ -139,18 +146,18 @@ public function get_raw_embed_dataset() {
139146
'
140147
<amp-facebook width="500" height="400" data-href="https://www.facebook.com/20531316728/posts/10154009990506729/" data-show-text="true" data-embed-as="post" layout="responsive">
141148
<blockquote cite="https://developers.facebook.com/20531316728/posts/10154009990506729/" class="fb-xfbml-parse-ignore" fallback="">
142-
Posted by <a href="https://www.facebook.com/facebook/">Facebook</a> on <a href="https://developers.facebook.com/20531316728/posts/10154009990506729/">Thursday, August 27, 2015</a>
149+
<p> Posted by <a href="https://www.facebook.com/facebook/">Facebook</a> on <a href="https://developers.facebook.com/20531316728/posts/10154009990506729/">Thursday, August 27, 2015</a></p>
143150
</blockquote>
144151
</amp-facebook>
145152
',
146153
],
147154

148-
'video_embed' => [
155+
'video_embed' => [
149156
'<div class="fb-video" data-href="https://www.facebook.com/amanda.orr.56/videos/10212156330049017/" data-show-text="false"></div>',
150157
'<amp-facebook width="600" height="400" data-href="https://www.facebook.com/amanda.orr.56/videos/10212156330049017/" data-show-text="false" data-embed-as="video" layout="responsive"></amp-facebook>',
151158
],
152159

153-
'page_embed' => [
160+
'page_embed' => [
154161
'
155162
<div class="fb-page" data-href="https://www.facebook.com/xwp.co/" data-width="340" data-height="432" data-hide-cover="true" data-show-facepile="true" data-show-posts="false">
156163
<div class="fb-xfbml-parse-ignore">
@@ -161,13 +168,15 @@ public function get_raw_embed_dataset() {
161168
'
162169
<amp-facebook-page width="340" height="432" data-href="https://www.facebook.com/xwp.co/" data-hide-cover="true" data-show-facepile="true" data-show-posts="false" layout="responsive">
163170
<div class="fb-xfbml-parse-ignore" fallback="">
164-
<blockquote cite="https://www.facebook.com/xwp.co/"><a href="https://www.facebook.com/xwp.co/">Like Us</a></blockquote>
171+
<blockquote cite="https://www.facebook.com/xwp.co/">
172+
<p><a href="https://www.facebook.com/xwp.co/">Like Us</a></p>
173+
</blockquote>
165174
</div>
166175
</amp-facebook-page>
167176
',
168177
],
169178

170-
'like' => [
179+
'like' => [
171180
'
172181
<div class="fb-like" data-href="https://developers.facebook.com/docs/plugins/" data-width="400" data-layout="standard" data-action="like" data-size="small" data-show-faces="true" data-share="true"></div>
173182
',
@@ -177,38 +186,53 @@ public function get_raw_embed_dataset() {
177186
',
178187
],
179188

180-
'comments' => [
189+
'comments' => [
181190
'
182191
<div class="fb-comments" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-numposts="5"></div>
183192
',
184193
'<amp-facebook-comments width="600" height="400" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-numposts="5" layout="responsive"></amp-facebook-comments>',
185194
],
186195

187-
'comments_full_width' => [
196+
'comments_full_width' => [
188197
'
189198
<div class="fb-comments" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-width="100%" data-numposts="5"></div>
190199
',
191200
'<amp-facebook-comments width="auto" height="400" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-numposts="5" layout="fixed-height"></amp-facebook-comments>',
192201
],
193202

194-
'comments_full_width_2' => [
203+
'comments_full_width_2' => [
195204
'
196205
<div class="fb-comments" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-height="123" data-width="100%" data-numposts="5"></div>
197206
',
198207
'<amp-facebook-comments width="auto" height="123" data-href="https://developers.facebook.com/docs/plugins/comments#configurator" data-numposts="5" layout="fixed-height"></amp-facebook-comments>',
199208
],
200209

201-
'comment_embed' => [
210+
'comment_embed' => [
202211
'
203212
<div class="fb-comment-embed" data-href="https://www.facebook.com/zuck/posts/10102735452532991?comment_id=1070233703036185" data-width="500"></div>
204213
',
205214
'<amp-facebook width="500" height="400" data-href="https://www.facebook.com/zuck/posts/10102735452532991?comment_id=1070233703036185" data-embed-as="comment" layout="responsive"></amp-facebook>',
206215
],
207216

208-
'remove_fb_root' => [
209-
'<div id="fb-root"></div><script async defer crossorigin="anonymous" src="https://connect.facebook.net/en_US/sdk.js#xfbml=1&version=v3.2"></script>', // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
217+
'remove_fb_root' => [
218+
'<div id="fb-root"></div>' . str_repeat( '<script async defer crossorigin="anonymous" src="https://connect.facebook.net/en_US/sdk.js#xfbml=1&version=v3.2"></script>', 5 ), // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
210219
'',
211220
],
221+
222+
'remove_multiple_fb_root' => [
223+
str_repeat( '<div id="fb-root"></div>', 5 ) . str_repeat( '<script async defer crossorigin="anonymous" src="https://connect.facebook.net/en_US/sdk.js#xfbml=1&version=v3.2"></script>', 5 ), // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
224+
'',
225+
],
226+
227+
'remove_empty_p_tag' => [
228+
'<p><script async defer crossorigin="anonymous" src="https://connect.facebook.net/en_US/sdk.js#xfbml=1&version=v3.2"></script></p><div id="fb-root"></div>', // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
229+
'',
230+
],
231+
232+
'keep_p_tag_if_it_has_children' => [
233+
'<p><span id="foo-bar"></span><script async defer crossorigin="anonymous" src="https://connect.facebook.net/en_US/sdk.js#xfbml=1&version=v3.2"></script></p><div id="fb-root"></div>', // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
234+
'<p><span id="foo-bar"></span></p>',
235+
],
212236
];
213237
}
214238

@@ -221,9 +245,11 @@ public function get_raw_embed_dataset() {
221245
* @covers AMP_Facebook_Embed_Handler::sanitize_raw_embeds()
222246
*/
223247
public function test__raw_embed_sanitizer( $source, $expected ) {
224-
$dom = AMP_DOM_Utils::get_dom_from_content( $source );
225248
$embed = new AMP_Facebook_Embed_Handler();
249+
$embed->register_embed();
226250

251+
$filtered_content = apply_filters( 'the_content', $source );
252+
$dom = AMP_DOM_Utils::get_dom_from_content( $filtered_content );
227253
$embed->sanitize_raw_embeds( $dom );
228254

229255
$layout_sanitizer = new AMP_Layout_Sanitizer( $dom );

0 commit comments

Comments
 (0)