Skip to content
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

Merged
merged 7 commits into from Jun 20, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ function amp_get_content_sanitizers( $post = null ) {
'AMP_Form_Sanitizer' => array(),
'AMP_Comments_Sanitizer' => array(),
'AMP_Video_Sanitizer' => array(),
'AMP_O2_Player_Sanitizer' => array(),
'AMP_Audio_Sanitizer' => array(),
'AMP_Playbuzz_Sanitizer' => array(),
'AMP_Iframe_Sanitizer' => array(
Expand Down
1 change: 1 addition & 0 deletions includes/class-amp-autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class AMP_Autoloader {
'AMP_Img_Sanitizer' => 'includes/sanitizers/class-amp-img-sanitizer',
'AMP_Comments_Sanitizer' => 'includes/sanitizers/class-amp-comments-sanitizer',
'AMP_Form_Sanitizer' => 'includes/sanitizers/class-amp-form-sanitizer',
'AMP_O2_Player_Sanitizer' => 'includes/sanitizers/class-amp-o2-player-sanitizer',
'AMP_Playbuzz_Sanitizer' => 'includes/sanitizers/class-amp-playbuzz-sanitizer',
'AMP_Style_Sanitizer' => 'includes/sanitizers/class-amp-style-sanitizer',
'AMP_Tag_And_Attribute_Sanitizer' => 'includes/sanitizers/class-amp-tag-and-attribute-sanitizer',
Expand Down
137 changes: 137 additions & 0 deletions includes/sanitizers/class-amp-o2-player-sanitizer.php
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>
*
Copy link
Member

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)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* @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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add phpdoc for this.

Copy link
Member

Choose a reason for hiding this comment

The 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 $matches.


/**
* 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be O2 not 02.

*
* @var string
*/
private static $height = '270';

/**
* Width to set for 02 Player elements.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be O2 not 02.

*
* @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 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The xpath setting here could just as well be moved to the sanitize method since that is where it is used. This would avoid needing to override the constructor at all, and if the sanitizers are instantiated but never call sanitize then we could avoid needlessly instantiating this object (however rare that may be).

}

/**
* 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();
}

}
101 changes: 101 additions & 0 deletions tests/test-amp-o2-player-sanitizer.php
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 );
}
}