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

Address issue where <ul> is converted to an <amp-carousel> #1529

Merged
merged 4 commits into from
Oct 24, 2018
Merged

Address issue where <ul> is converted to an <amp-carousel> #1529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e68f194
Address issue where <ul> with an <a> is converted to an <amp-carousel>
kienstra Oct 24, 2018
823fd0e
Add workaround for build error in testing WP 5.0
westonruter Oct 24, 2018
eb6fb1b
Move block-library/style.css creation to install step
westonruter Oct 24, 2018
4a4fe0c
Move workaround to install_wp in dev-lib
westonruter Oct 24, 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
2 changes: 1 addition & 1 deletion dev-lib
Open in desktop
Submodule dev-lib updated 1 files
+4 −0 check-diff.sh
16 changes: 13 additions & 3 deletions includes/sanitizers/class-amp-gallery-block-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,21 @@ class AMP_Gallery_Block_Sanitizer extends AMP_Base_Sanitizer {
/**
* Tag.
*
* @var string Ul tag to identify wrapper around gallery block.
* @since 1.0
*
* @var string Ul tag to identify wrapper around gallery block.
*/
public static $tag = 'ul';

/**
* Expected class of the wrapper around the gallery block.
*
* @since 1.0
*
* @var string
*/
public static $class = 'wp-block-gallery';

/**
* Array of flags used to control sanitization.
*
Expand Down Expand Up @@ -72,8 +82,8 @@ public function sanitize() {
for ( $i = $num_nodes - 1; $i >= 0; $i-- ) {
$node = $nodes->item( $i );

// We're looking for <ul> elements that at least one child.
if ( 0 === count( $node->childNodes ) ) {
// We're looking for <ul> elements that have at least one child and the proper class.
if ( 0 === count( $node->childNodes ) || false === strpos( $node->getAttribute( 'class' ), self::$class ) ) {
continue;
}

Expand Down
73 changes: 66 additions & 7 deletions tests/test-class-amp-gallery-block-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,23 @@ public function get_data() {
'<ul><a><amp-img></amp-img></a></ul>',
),

'data_amp_with_carousel' => array(
'no_block_class' => array(
'<ul data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<ul data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
),

'data_amp_with_carousel' => array(
'<ul class="wp-block-gallery" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></amp-carousel>',
),

'data_amp_with_lightbox' => array(
'<ul data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<ul data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></a></figure></li></ul><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></a></figure></li></ul><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),

'data_amp_with_lightbox_and_carousel' => array(
'<ul data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),
);
Expand All @@ -52,15 +57,69 @@ public function get_data() {
/**
* Test sanitizer.
*
* This only tests when theme support is present.
* Like if Native or Paired is selected in AMP Settings > Template Mode,
* or if this is added with add_theme_support( 'amp' ).
* If there is no theme support, the sanitizer will have the argument array( 'carousel_required' => true ).
*
* @see amp_get_content_sanitizers()
* @dataProvider get_data
* @param string $source Source.
* @param string $expected Expected.
*/
public function test_sanitizer( $source, $expected ) {
$dom = AMP_DOM_Utils::get_dom_from_content( $source );
$sanitizer = new AMP_Gallery_Block_Sanitizer( $dom, array(
'content_max_width' => 600,
) );
$sanitizer = new AMP_Gallery_Block_Sanitizer(
$dom,
array( 'content_max_width' => 600 )
);
$sanitizer->sanitize();
$content = AMP_DOM_Utils::get_content_from_dom( $dom );
$content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content );
$this->assertEquals( $expected, $content );
}

/**
* Get the Classic mode data.
*
* @return array
*/
public function get_classic_mode_data() {
return array(
'no_block_class' => array(
'<ul data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<ul data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
),

'data_amp_with_lightbox' => array(
'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),

'data_amp_with_lightbox_and_carousel' => array(
'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),
);
}

/**
* Test the sanitizer in Classic mode (without theme support).
*
* The tested sanitizer will have an argument of array( 'carousel_required' => true ),
* which sometimes causes different output.
*
* @see amp_get_content_sanitizers()
* @dataProvider get_classic_mode_data
* @param string $source Source.
* @param string $expected Expected.
*/
public function test_sanitizer_classic_mode( $source, $expected ) {
$dom = AMP_DOM_Utils::get_dom_from_content( $source );
$sanitizer = new AMP_Gallery_Block_Sanitizer(
$dom,
array( 'carousel_required' => true )
);
$sanitizer->sanitize();
$content = AMP_DOM_Utils::get_content_from_dom( $dom );
$content = preg_replace( '/(?<=>)\s+(?=<)/', '', $content );
Expand Down