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.

Sign up for GitHub

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

Jump to bottom

Update serialization to use HTML instead of XML #891

Merged
merged 3 commits into from
Jan 24, 2018
Merged

Update serialization to use HTML instead of XML #891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9efa282
Bump minimum version of PHP to 5.3
westonruter Jan 23, 2018
ed336c8
Fully deprecate blacklist sanitizer and remove tests
westonruter Jan 23, 2018
a0ceeac
Use DOMDocument::saveHTML() instead of saveXML
westonruter Jan 23, 2018
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
4 changes: 0 additions & 4 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ cache:

matrix:
include:
- php: "5.2"
env: WP_VERSION=latest DEV_LIB_SKIP=phpcs
- php: "5.2"
env: WP_VERSION=4.7 DEV_LIB_SKIP=phpcs
- php: "5.3"
env: WP_VERSION=latest DEV_LIB_SKIP=phpcs
- php: "5.3"
Expand Down
17 changes: 17 additions & 0 deletions amp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@
* @package AMP
*/

/**
* Print admin notice regarding having an old version of PHP.
*
* @since 0.7
*/
function _amp_print_php_version_admin_notice() {
?>
<div class="notice notice-error">
<p><?php esc_html_e( 'The AMP plugin requires PHP 5.3+. Please contact your host to update your PHP version.', 'amp' ); ?></p>
</div>
<?php
}
if ( version_compare( phpversion(), '5.3', '<' ) ) {
add_action( 'admin_notices', '_amp_print_php_version_admin_notice' );
return;
}

define( 'AMP__FILE__', __FILE__ );
define( 'AMP__DIR__', dirname( __FILE__ ) );
define( 'AMP__VERSION', '0.7-alpha' );
Expand Down
2 changes: 2 additions & 0 deletions includes/sanitizers/class-amp-blacklist-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class AMP_Blacklist_Sanitizer extends AMP_Base_Sanitizer {
* Sanitize.
*/
public function sanitize() {
_deprecated_function( __METHOD__, '0.7', 'AMP_Tag_And_Attribute_Sanitizer::sanitize' );

$blacklisted_tags = $this->get_blacklisted_tags();
$blacklisted_attributes = $this->get_blacklisted_attributes();
$blacklisted_protocols = $this->get_blacklisted_protocols();
Expand Down
17 changes: 5 additions & 12 deletions includes/utils/class-amp-dom-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,23 +160,15 @@ public static function get_content_from_dom_node( $dom, $node ) {
*/
static $self_closing_tags_regex;

/*
* Most AMP elements need closing tags. To force them, we cannot use
* saveHTML (node support is 5.3+) and LIBXML_NOEMPTYTAG results in
* issues with self-closing tags like `br` and `hr`. So, we're manually
* forcing closing tags.
*/
self::recursive_force_closing_tags( $dom, $node );

/*
* Cache this regex so we don't have to recreate it every call.
*/
if ( ! isset( $self_closing_tags_regex ) ) {
$self_closing_tags = implode( '|', self::$self_closing_tags );
$self_closing_tags_regex = "#></({$self_closing_tags})>#i";
$self_closing_tags_regex = "#</({$self_closing_tags})>#i";
}

$html = $dom->saveXML( $node );
$html = $dom->saveHTML( $node );

// Whitespace just causes unit tests to fail... so whitespace begone.
if ( '' === trim( $html ) ) {
Expand All @@ -189,10 +181,9 @@ public static function get_content_from_dom_node( $dom, $node ) {
* Seems like LIBXML_NOEMPTYTAG was passed, but as you can see it was not.
* This does not happen in my (@mikeschinkel) local testing, btw.
*/
$html = preg_replace( $self_closing_tags_regex, '/>', $html );
$html = preg_replace( $self_closing_tags_regex, '', $html );

return $html;

}

/**
Expand Down Expand Up @@ -268,12 +259,14 @@ public static function is_node_empty( $node ) {
* Forces HTML element closing tags given a DOMDocument and optional DOMElement
*
* @since 0.2
* @deprecated
*
* @param DOMDocument $dom Represents HTML document on which to force closing tags.
* @param DOMElement $node Represents HTML element to start closing tags on.
* If not passed, defaults to first child of body.
*/
public static function recursive_force_closing_tags( $dom, $node = null ) {
_deprecated_function( __METHOD__, '0.7' );

if ( is_null( $node ) ) {
$node = $dom->getElementsByTagName( 'body' )->item( 0 );
Expand Down
2 changes: 1 addition & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Enable Accelerated Mobile Pages (AMP) on your WordPress site.
**Tested up to:** 4.9
**Stable tag:** 0.6.0
**License:** [GPLv2 or later](http://www.gnu.org/licenses/gpl-2.0.html)
**Requires PHP:** 5.2
**Requires PHP:** 5.3

[![Build Status](https://travis-ci.org/Automattic/amp-wp.svg?branch=master)](https://travis-ci.org/Automattic/amp-wp) [![Built with Grunt](https://cdn.gruntjs.com/builtwith.svg)](http://gruntjs.com)

Expand Down
2 changes: 1 addition & 1 deletion readme.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Tested up to: 4.9
Stable tag: 0.6.0
License: GPLv2 or later
License URI: http://www.gnu.org/licenses/gpl-2.0.html
Requires PHP: 5.2
Requires PHP: 5.3

Enable Accelerated Mobile Pages (AMP) on your WordPress site.

Expand Down
11 changes: 6 additions & 5 deletions tests/amp-tag-and-attribute-sanitizer-private-methods-tests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,10 +619,10 @@ public function get_remove_node_data() {
),
'remove_bad_tag_leave_empty_siblings_and_parent' => array(
array(
'source' => '<div><br/><bad-tag></bad-tag></div>',
'source' => '<div><br><bad-tag></bad-tag></div>',
'tag_name' => 'bad-tag',
),
'<div><br/></div>',
'<div><br></div>',
),
'remove_single_bad_tag_with_non-empty_parent' => array(
array(
Expand Down Expand Up @@ -759,10 +759,10 @@ public function get_replace_node_with_children_data() {
),
'no_children_leave_empty_siblings_and_parent' => array(
array(
'source' => '<div><br/><bad-tag></bad-tag></div>',
'source' => '<div><br><bad-tag></bad-tag></div>',
'tag_name' => 'bad-tag',
),
'<div><br/></div>',
'<div><br></div>',
),
'no_childreng_with_non-empty_parent' => array(
array(
Expand All @@ -780,7 +780,7 @@ public function get_replace_node_with_children_data() {
),
'no_children_and_empty_grandparent_leave_non-empty_greatgrandparent' => array(
array(
'source' => '<div><div><div><bad-tag></bad-tag></div></div><p>Good Data</p></div>',
'source' => '<div><div><div><bad-tag></bad-tag></div></div><p>Good Data</p></div>',
'tag_name' => 'bad-tag',
),
'<div><p>Good Data</p></div>',
Expand All @@ -800,6 +800,7 @@ public function test_replace_node_with_children( $data, $expected ) {
$this->invoke_method( $sanitizer, 'replace_node_with_children', array( $node ) );

$got = AMP_DOM_Utils::get_content_from_dom( $dom );
$got = preg_replace( '/(?<=>)\s+(?=<)/', '', $got );

if ( $expected != $got ) {
printf( 'using source: %s' . PHP_EOL, $data['source'] );
Expand Down
7 changes: 4 additions & 3 deletions tests/test-amp-audio-converter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function get_data() {
'<audio width="400" height="300">
<source src="https://example.com/foo.wav" type="audio/wav">
</audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"/></amp-audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"></amp-audio>',
),

'multiple_same_audio' => array(
Expand All @@ -67,7 +67,7 @@ public function get_data() {
<audio width="400" height="300">
<source src="https://example.com/foo.wav" type="audio/wav">
</audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"/></amp-audio><amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"/></amp-audio><amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"/></amp-audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"></amp-audio><amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"></amp-audio><amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"></amp-audio>',
),

'multiple_different_audio' => array(
Expand All @@ -78,7 +78,7 @@ public function get_data() {
<audio height="500" width="300">
<source src="https://example.com/foo2.wav" type="audio/wav">
</audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"/></amp-audio><amp-audio width="400" height="300" src="https://example.com/audio/file.ogg"></amp-audio><amp-audio height="500" width="300"><source src="https://example.com/foo2.wav" type="audio/wav"/></amp-audio>',
'<amp-audio width="400" height="300"><source src="https://example.com/foo.wav" type="audio/wav"></amp-audio><amp-audio width="400" height="300" src="https://example.com/audio/file.ogg"></amp-audio><amp-audio height="500" width="300"><source src="https://example.com/foo2.wav" type="audio/wav"></amp-audio>',
),

'https_not_required' => array(
Expand All @@ -96,6 +96,7 @@ public function test_converter( $source, $expected ) {
$sanitizer = new AMP_Audio_Sanitizer( $dom );
$sanitizer->sanitize();
$content = AMP_DOM_Utils::get_content_from_dom( $dom );
$content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content );
$this->assertEquals( $expected, $content );
}

Expand Down
194 changes: 0 additions & 194 deletions tests/test-amp-blacklist-sanitizer.php
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions tests/test-amp-iframe-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public function test_converter( $source, $expected ) {
$sanitizer = new AMP_Iframe_Sanitizer( $dom );
$sanitizer->sanitize();
$content = AMP_DOM_Utils::get_content_from_dom( $dom );
$content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content );
$this->assertEquals( $expected, $content );
}

Expand Down
Loading