-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding sanitizer to support amp-o2-player #1202
Changes from 2 commits
f46490e
1dcb923
bd1ff45
ca1f589
b0589b1
2909bb0
cc5606e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
<?php | ||
/** | ||
* Class AMP_O2_Player_Sanitizer | ||
* | ||
* @package AMP | ||
*/ | ||
|
||
/** | ||
* Class AMP_O2_Player_Sanitizer | ||
* | ||
* Converts <div class="vdb_player><script></script></div> embed to <amp-o2-player> | ||
* | ||
* @see https://www.ampproject.org/docs/reference/components/amp-o2-player | ||
*/ | ||
class AMP_O2_Player_Sanitizer extends AMP_Base_Sanitizer { | ||
const URL_PATTERN = '#.*delivery.vidible.tv\/jsonp\/pid=(.*)\/vid=(.*)\/(.*).js.*#i'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be good to add phpdoc for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might want to use named capture groups to make it easier to reference the |
||
|
||
/** | ||
* AMP Tag. | ||
* | ||
* @var string AMP Tag. | ||
*/ | ||
public static $amp_tag = 'amp-o2-player'; | ||
|
||
/** | ||
* Amp O2 Player class. | ||
* | ||
* @var string CSS class to identify O2 Player <div> to replace with AMP version. | ||
*/ | ||
public static $xpath_selector = '//div[ contains( @class, \'vdb_player\' ) ]/script'; | ||
|
||
/** | ||
* Height to set for 02 Player elements. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
* | ||
* @var string | ||
*/ | ||
private static $height = '270'; | ||
|
||
/** | ||
* Width to set for 02 Player elements. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be |
||
* | ||
* @var string | ||
*/ | ||
private static $width = '480'; | ||
|
||
/** | ||
* XPath. | ||
* | ||
* @var DOMXPath | ||
*/ | ||
private $xpath; | ||
|
||
/** | ||
* AMP_O2_Player_Sanitizer constructor. | ||
* | ||
* @param DOMDocument $dom Represents the HTML document to sanitize. | ||
* @param array $args Args. | ||
*/ | ||
public function __construct( DOMDocument $dom, array $args = array() ) { | ||
parent::__construct( $dom, $args ); | ||
$this->xpath = new DOMXPath( $dom ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
} | ||
|
||
/** | ||
* Sanitize the O2 Player elements from the HTML contained in this instance's DOMDocument. | ||
*/ | ||
public function sanitize() { | ||
/** | ||
* Node list. | ||
* | ||
* @var DOMNodeList $nodes | ||
*/ | ||
$nodes = $this->xpath->query( self::$xpath_selector ); | ||
$num_nodes = $nodes->length; | ||
|
||
if ( 0 === $num_nodes ) { | ||
return; | ||
} | ||
|
||
for ( $i = $num_nodes - 1; $i >= 0; $i-- ) { | ||
$node = $nodes->item( $i ); | ||
|
||
$this->create_amp_o2_player( $this->dom, $node ); | ||
} | ||
|
||
} | ||
|
||
/** | ||
* Replaces node with amp-o2-player | ||
* | ||
* @param DOMDocument $dom The HTML Document. | ||
* @param DOMNode $node The DOMNode to adjust and replace. | ||
*/ | ||
private function create_amp_o2_player( $dom, $node ) { | ||
$o2_attributes = $this->get_o2_player_attributes( $node->getAttribute( 'src' ) ); | ||
|
||
if ( ! empty( $o2_attributes ) ) { | ||
$component_attributes = array_merge( | ||
$o2_attributes, array( | ||
'data-macros' => 'm.playback=click', | ||
'layout' => 'responsive', | ||
'width' => self::$width, | ||
'height' => self::$height, | ||
) | ||
); | ||
|
||
$amp_o2_player = AMP_DOM_Utils::create_node( $dom, self::$amp_tag, $component_attributes ); | ||
|
||
$parent_node = $node->parentNode; | ||
|
||
// replaces the wrapper that contains the script with amp-o2-player element. | ||
$parent_node->parentNode->replaceChild( $amp_o2_player, $parent_node ); | ||
|
||
$this->did_convert_elements = true; | ||
} | ||
} | ||
|
||
/** | ||
* Gets O2 Player's required attributes from script src | ||
* | ||
* @param string $src script src. | ||
* | ||
* @return array data-attributes for o2 player. | ||
*/ | ||
private function get_o2_player_attributes( $src ) { | ||
$found = preg_match( self::URL_PATTERN, $src, $matches ); | ||
if ( $found ) { | ||
return array( | ||
'data-pid' => $matches[1], | ||
'data-vid' => $matches[2], | ||
'data-bcid' => $matches[3], | ||
); | ||
} | ||
return array(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
<?php | ||
/** | ||
* Test AMP AMP_O2_Player_Sanitizer. | ||
* | ||
* @package AMP | ||
*/ | ||
|
||
/** | ||
* Test AMP_O2_Player_Sanitizer | ||
*/ | ||
class AMP_O2_Player_Sanitizer_Test extends WP_UnitTestCase { | ||
|
||
/** | ||
* Data for converter test. | ||
* | ||
* @return array Data. | ||
*/ | ||
public function get_data() { | ||
return array( | ||
'no_o2_player_item' => array( | ||
'<h1>Im Not A O2 Player</h1>', | ||
'<h1>Im Not A O2 Player</h1>', | ||
), | ||
'o2_player_item_without_script' => array( | ||
'<div class="vdb_player"></div>', | ||
'<div class="vdb_player"></div>', | ||
), | ||
|
||
'o2_player_item' => array( | ||
'<div class="vdb_player"><script type="text/javascript" src="//delivery.vidible.tv/jsonp/pid=59521379f3bdc970c5c9d75e/vid=5b11a50d0239e257abfdf16a/59521191e9399f3a7d7de88f.js?m.embeded=cms_video_plugin_chromeExtension"></script></div>',// phpcs:ignore | ||
'<amp-o2-player data-pid="59521379f3bdc970c5c9d75e" data-vid="5b11a50d0239e257abfdf16a" data-bcid="59521191e9399f3a7d7de88f" data-macros="m.playback=click" layout="responsive" width="480" height="270"></amp-o2-player>', | ||
), | ||
|
||
'o2_player_item_without_required_fields' => array( | ||
'<div class="vdb_player"><script type="text/javascript" src="//delivery.vidible.tv/jsonp/vid=5b11a50d0239e257abfdf16a/59521191e9399f3a7d7de88f.js?m.embeded=cms_video_plugin_chromeExtension"></script></div>',// phpcs:ignore | ||
'<div class="vdb_player"><script type="text/javascript" src="//delivery.vidible.tv/jsonp/vid=5b11a50d0239e257abfdf16a/59521191e9399f3a7d7de88f.js?m.embeded=cms_video_plugin_chromeExtension"></script></div>',// phpcs:ignore | ||
), | ||
|
||
); | ||
} | ||
|
||
/** | ||
* Dataset to test amp-o2-player sanitizer/ | ||
* | ||
* @param string $source Content. | ||
* @param string $expected Expected content. | ||
* | ||
* @dataProvider get_data | ||
*/ | ||
public function test_converter( $source, $expected ) { | ||
$dom = AMP_DOM_Utils::get_dom_from_content( $source ); | ||
$sanitizer = new AMP_O2_Player_Sanitizer( $dom ); | ||
|
||
$sanitizer->sanitize(); | ||
|
||
$content = AMP_DOM_Utils::get_content_from_dom( $dom ); | ||
|
||
$this->assertEquals( $expected, $content ); | ||
} | ||
|
||
/** | ||
* Tests get script when required attributes missing. | ||
*/ | ||
public function test_get_script__pid__vid__required() { | ||
$source = '<div class="vdb_player"><script type="text/javascript" src="//delivery.vidible.tv/jsonp/pid=59521379f3bdc970c5c9d75e?m.embeded=cms_video_plugin_chromeExtension"></script></div>';// phpcs:ignore | ||
$expected = array(); | ||
|
||
$dom = AMP_DOM_Utils::get_dom_from_content( $source ); | ||
$sanitizer = new AMP_O2_Player_Sanitizer( $dom ); | ||
$sanitizer->sanitize(); | ||
|
||
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); | ||
$whitelist_sanitizer->sanitize(); | ||
|
||
$scripts = array_merge( | ||
$sanitizer->get_scripts(), | ||
$whitelist_sanitizer->get_scripts() | ||
); | ||
$this->assertEquals( $expected, $scripts ); | ||
} | ||
|
||
/** | ||
* Test that get_scripts() did convert. | ||
*/ | ||
public function test_get_scripts__did_convert() { | ||
$source = '<div class="vdb_player"><script type="text/javascript" src="//delivery.vidible.tv/jsonp/pid=59521379f3bdc970c5c9d75e/vid=5b11a50d0239e257abfdf16a/59521191e9399f3a7d7de88f.js?m.embeded=cms_video_plugin_chromeExtension"></script></div>';// phpcs:ignore | ||
$expected = array( 'amp-o2-player' => true ); | ||
|
||
$dom = AMP_DOM_Utils::get_dom_from_content( $source ); | ||
$sanitizer = new AMP_O2_Player_Sanitizer( $dom ); | ||
$sanitizer->sanitize(); | ||
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom ); | ||
$whitelist_sanitizer->sanitize(); | ||
|
||
$scripts = array_merge( | ||
$sanitizer->get_scripts(), | ||
$whitelist_sanitizer->get_scripts() | ||
); | ||
$this->assertEquals( $expected, $scripts ); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add
@since 1.0
tag to the class (at least)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done