From e98cb09c1ff3d2c22f59bc58ea9cb9e4202ce8e9 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Mon, 29 Jan 2018 18:25:01 -0600 Subject: [PATCH 1/5] Issue #864: Support in 'Gallery' widget. There's an existing handler to create 'amp-carousel' elements: class AMP_Gallery_Embed_Handler. So override the 'Gallery' widget class. And use that in render_media(). Otherwise, that function is copied from the parent. The parent calls gallery_shortcode() at the end of render_media(). And that function doesn't have a filter for the markup. --- includes/class-amp-autoloader.php | 1 + .../class-amp-widget-media-gallery.php | 51 +++++++++++++ tests/test-class-amp-widget-media-gallery.php | 74 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 includes/widgets/class-amp-widget-media-gallery.php create mode 100644 tests/test-class-amp-widget-media-gallery.php diff --git a/includes/class-amp-autoloader.php b/includes/class-amp-autoloader.php index b0904dd69e8..ee5e0bd9a82 100644 --- a/includes/class-amp-autoloader.php +++ b/includes/class-amp-autoloader.php @@ -86,6 +86,7 @@ class AMP_Autoloader { 'AMP_WP_Utils' => 'includes/utils/class-amp-wp-utils', 'AMP_Widget_Archives' => 'includes/widgets/class-amp-widget-archives', 'AMP_Widget_Categories' => 'includes/widgets/class-amp-widget-categories', + 'AMP_Widget_Media_Gallery' => 'includes/widgets/class-amp-widget-media-gallery', 'AMP_Widget_Recent_Comments' => 'includes/widgets/class-amp-widget-recent-comments', 'WPCOM_AMP_Polldaddy_Embed' => 'wpcom/class-amp-polldaddy-embed', 'AMP_Test_Stub_Sanitizer' => 'tests/stubs', diff --git a/includes/widgets/class-amp-widget-media-gallery.php b/includes/widgets/class-amp-widget-media-gallery.php new file mode 100644 index 00000000000..9bd645d0962 --- /dev/null +++ b/includes/widgets/class-amp-widget-media-gallery.php @@ -0,0 +1,51 @@ +get_instance_schema(), 'default' ), $instance ); + $shortcode_atts = array_merge( + $instance, + array( + 'link' => $instance['link_type'], + ) + ); + + if ( isset( $instance['orderby_random'] ) && ( true === $instance['orderby_random'] ) ) { + $shortcode_atts['orderby'] = 'rand'; + } + + $handler = new AMP_Gallery_Embed_Handler(); + echo $handler->shortcode( $shortcode_atts ); // WPCS: XSS ok. + } + +} diff --git a/tests/test-class-amp-widget-media-gallery.php b/tests/test-class-amp-widget-media-gallery.php new file mode 100644 index 00000000000..5f374b19f38 --- /dev/null +++ b/tests/test-class-amp-widget-media-gallery.php @@ -0,0 +1,74 @@ +instance = new AMP_Widget_Media_Gallery(); + } + + /** + * Test render_media(). + * + * @see AMP_Widget_Media_Gallery::widget(). + */ + public function test_render_media() { + $first_test_image = '/tmp/test-image.jpg'; + copy( DIR_TESTDATA . '/images/test-image.jpg', $first_test_image ); + $first_attachment_id = self::factory()->attachment->create_object( array( + 'file' => $first_test_image, + 'post_parent' => 0, + 'post_mime_type' => 'image/jpeg', + 'post_title' => 'Test Image', + ) ); + wp_update_attachment_metadata( $first_attachment_id, wp_generate_attachment_metadata( $first_attachment_id, $first_test_image ) ); + $ids[] = $first_attachment_id; + + $second_test_image = '/tmp/test-image.jpg'; + copy( DIR_TESTDATA . '/images/test-image.jpg', $second_test_image ); + $second_attachment_id = self::factory()->attachment->create_object( array( + 'file' => $second_test_image, + 'post_parent' => 0, + 'post_mime_type' => 'image/jpeg', + 'post_title' => 'Test Image', + ) ); + wp_update_attachment_metadata( $second_attachment_id, wp_generate_attachment_metadata( $second_attachment_id, $second_test_image ) ); + $ids[] = $second_attachment_id; + $instance = array( + 'title' => 'Test Gallery Widget', + 'ids' => $ids, + ); + + ob_start(); + $this->instance->render_media( $instance ); + $output = ob_get_clean(); + + $this->assertContains( 'amp-carousel', $output ); + $this->assertContains( $first_test_image, $output ); + $this->assertContains( $second_test_image, $output ); + } + +} From a8b4bf9a72c2037ce123934502499c3901bec667 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Mon, 29 Jan 2018 18:55:36 -0600 Subject: [PATCH 2/5] Issue #864: Accomodate WP versions < 4.9, without the 'Gallery' widget. Don't subclass the widget if it doesn't exist. And don't test the subclass if it's not present. --- .../class-amp-widget-media-gallery.php | 68 ++++++++++--------- tests/test-class-amp-widget-media-gallery.php | 3 + 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/includes/widgets/class-amp-widget-media-gallery.php b/includes/widgets/class-amp-widget-media-gallery.php index 9bd645d0962..f0036eda9c7 100644 --- a/includes/widgets/class-amp-widget-media-gallery.php +++ b/includes/widgets/class-amp-widget-media-gallery.php @@ -6,46 +6,48 @@ * @package AMP */ -/** - * Class AMP_Widget_Media_Gallery - * - * @since 0.7.0 - * @package AMP - */ -class AMP_Widget_Media_Gallery extends WP_Widget_Media_Gallery { - +if ( class_exists( 'WP_Widget_Media_Gallery' ) ) { /** - * Renders the markup of the widget. - * - * Mainly copied from WP_Widget_Media_Gallery::render_media(). - * But instead of calling shortcode_gallery(), - * It uses this plugin's embed handler for galleries. + * Class AMP_Widget_Media_Gallery * * @since 0.7.0 - * - * @param array $instance Data for widget. - * @return void + * @package AMP */ - public function render_media( $instance ) { - if ( ! is_amp_endpoint() ) { - parent::render_media( $instance ); - return; - } + class AMP_Widget_Media_Gallery extends WP_Widget_Media_Gallery { - $instance = array_merge( wp_list_pluck( $this->get_instance_schema(), 'default' ), $instance ); - $shortcode_atts = array_merge( - $instance, - array( - 'link' => $instance['link_type'], - ) - ); + /** + * Renders the markup of the widget. + * + * Mainly copied from WP_Widget_Media_Gallery::render_media(). + * But instead of calling shortcode_gallery(), + * It uses this plugin's embed handler for galleries. + * + * @since 0.7.0 + * + * @param array $instance Data for widget. + * @return void + */ + public function render_media( $instance ) { + if ( ! is_amp_endpoint() ) { + parent::render_media( $instance ); + return; + } - if ( isset( $instance['orderby_random'] ) && ( true === $instance['orderby_random'] ) ) { - $shortcode_atts['orderby'] = 'rand'; + $instance = array_merge( wp_list_pluck( $this->get_instance_schema(), 'default' ), $instance ); + $shortcode_atts = array_merge( + $instance, + array( + 'link' => $instance['link_type'], + ) + ); + + if ( isset( $instance['orderby_random'] ) && ( true === $instance['orderby_random'] ) ) { + $shortcode_atts['orderby'] = 'rand'; + } + + $handler = new AMP_Gallery_Embed_Handler(); + echo $handler->shortcode( $shortcode_atts ); // WPCS: XSS ok. } - $handler = new AMP_Gallery_Embed_Handler(); - echo $handler->shortcode( $shortcode_atts ); // WPCS: XSS ok. } - } diff --git a/tests/test-class-amp-widget-media-gallery.php b/tests/test-class-amp-widget-media-gallery.php index 5f374b19f38..c377b692616 100644 --- a/tests/test-class-amp-widget-media-gallery.php +++ b/tests/test-class-amp-widget-media-gallery.php @@ -28,6 +28,9 @@ public function setUp() { parent::setUp(); AMP_Theme_Support::register_widgets(); $this->instance = new AMP_Widget_Media_Gallery(); + if ( ! class_exists( 'WP_Widget_Media_Gallery' ) ) { + $this->markTestSkipped( 'This WordPress version does not have a Gallery widget' ); + } } /** From a2bd726b1c440b3c79e15ff40651cf2c523df78b Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Mon, 29 Jan 2018 19:03:08 -0600 Subject: [PATCH 3/5] Issue #864: Move the markTestSkipped() higher in the function. Before, there was an error, As it instantiated AMP_Widget_Media_Gallery if it didn't exist. --- tests/test-class-amp-widget-media-gallery.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test-class-amp-widget-media-gallery.php b/tests/test-class-amp-widget-media-gallery.php index c377b692616..281dd205adb 100644 --- a/tests/test-class-amp-widget-media-gallery.php +++ b/tests/test-class-amp-widget-media-gallery.php @@ -25,12 +25,12 @@ class Test_AMP_Widget_Media_Gallery extends WP_UnitTestCase { * @inheritdoc */ public function setUp() { + if ( ! class_exists( 'AMP_Widget_Media_Gallery' ) ) { + $this->markTestSkipped( 'This WordPress version does not have a Gallery widget.' ); + } parent::setUp(); AMP_Theme_Support::register_widgets(); $this->instance = new AMP_Widget_Media_Gallery(); - if ( ! class_exists( 'WP_Widget_Media_Gallery' ) ) { - $this->markTestSkipped( 'This WordPress version does not have a Gallery widget' ); - } } /** From 2aa7ba2e60ca5e02e1de24ec29389d7094586980 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 29 Jan 2018 17:36:27 -0800 Subject: [PATCH 4/5] Use do_shortcode() instead of calling gallery embed logic directly --- .../class-amp-widget-media-gallery.php | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/includes/widgets/class-amp-widget-media-gallery.php b/includes/widgets/class-amp-widget-media-gallery.php index f0036eda9c7..e836025a986 100644 --- a/includes/widgets/class-amp-widget-media-gallery.php +++ b/includes/widgets/class-amp-widget-media-gallery.php @@ -10,6 +10,8 @@ /** * Class AMP_Widget_Media_Gallery * + * @todo This subclass can eventually be eliminated once #25435 is merged and the WP_Widget_Media_Gallery::render_media() does do_single_shortcode( 'gallery', $instance ). + * @link https://core.trac.wordpress.org/ticket/25435 * @since 0.7.0 * @package AMP */ @@ -19,8 +21,7 @@ class AMP_Widget_Media_Gallery extends WP_Widget_Media_Gallery { * Renders the markup of the widget. * * Mainly copied from WP_Widget_Media_Gallery::render_media(). - * But instead of calling shortcode_gallery(), - * It uses this plugin's embed handler for galleries. + * But instead of calling shortcode_gallery(), it calls do_shortcode(). * * @since 0.7.0 * @@ -45,8 +46,21 @@ public function render_media( $instance ) { $shortcode_atts['orderby'] = 'rand'; } - $handler = new AMP_Gallery_Embed_Handler(); - echo $handler->shortcode( $shortcode_atts ); // WPCS: XSS ok. + /* + * The following calls do_shortcode() in case another plugin overrides the gallery shortcode. + * The AMP_Gallery_Embed_Handler in the plugin is doing this itself, but other plugins may + * do it as well, so this ensures that a plugin has the option to override the gallery behavior + * via registering a different gallery shortcode handler. The shortcode serialization can be + * eliminated once WP Trac #25435 is merged and the Gallery widget uses the proposed do_single_shortcode(). + */ + $shortcode_atts_str = ''; + if ( is_array( $shortcode_atts['ids'] ) ) { + $shortcode_atts['ids'] = join( ',', $shortcode_atts['ids'] ); + } + foreach ( $shortcode_atts as $key => $value ) { + $shortcode_atts_str .= sprintf( ' %s="%s"', $key, esc_attr( $value ) ); + } + echo do_shortcode( "[gallery $shortcode_atts_str]" ); // WPCS: XSS ok. } } From d6cccd27bc5b94847470afbffb7f6c6b04f6a8d0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 29 Jan 2018 19:14:55 -0800 Subject: [PATCH 5/5] Add links to images in gallery when requested --- includes/embeds/class-amp-gallery-embed.php | 109 ++++++++++++++------ 1 file changed, 75 insertions(+), 34 deletions(-) diff --git a/includes/embeds/class-amp-gallery-embed.php b/includes/embeds/class-amp-gallery-embed.php index 6f46325fb80..1261d38747b 100644 --- a/includes/embeds/class-amp-gallery-embed.php +++ b/includes/embeds/class-amp-gallery-embed.php @@ -7,17 +7,31 @@ /** * Class AMP_Gallery_Embed_Handler + * + * @since 0.2 */ class AMP_Gallery_Embed_Handler extends AMP_Base_Embed_Handler { + /** + * Register embed. + */ public function register_embed() { add_shortcode( 'gallery', array( $this, 'shortcode' ) ); } + /** + * Unregister embed. + */ public function unregister_embed() { remove_shortcode( 'gallery' ); } + /** + * Shortcode handler. + * + * @param array $attr Shortcode attributes. + * @return string Rendered gallery. + */ public function shortcode( $attr ) { $post = get_post(); @@ -30,46 +44,47 @@ public function shortcode( $attr ) { } $atts = shortcode_atts( array( - 'order' => 'ASC', - 'orderby' => 'menu_order ID', - 'id' => $post ? $post->ID : 0, - 'include' => '', - 'exclude' => '', - 'size' => array( $this->args['width'], $this->args['height'] ), + 'order' => 'ASC', + 'orderby' => 'menu_order ID', + 'id' => $post ? $post->ID : 0, + 'include' => '', + 'exclude' => '', + 'size' => array( $this->args['width'], $this->args['height'] ), + 'link' => 'none', ), $attr, 'gallery' ); $id = intval( $atts['id'] ); if ( ! empty( $atts['include'] ) ) { $attachments = get_posts( array( - 'include' => $atts['include'], - 'post_status' => 'inherit', - 'post_type' => 'attachment', + 'include' => $atts['include'], + 'post_status' => 'inherit', + 'post_type' => 'attachment', 'post_mime_type' => 'image', - 'order' => $atts['order'], - 'orderby' => $atts['orderby'], - 'fields' => 'ids', + 'order' => $atts['order'], + 'orderby' => $atts['orderby'], + 'fields' => 'ids', ) ); } elseif ( ! empty( $atts['exclude'] ) ) { $attachments = get_children( array( - 'post_parent' => $id, - 'exclude' => $atts['exclude'], - 'post_status' => 'inherit', - 'post_type' => 'attachment', + 'post_parent' => $id, + 'exclude' => $atts['exclude'], + 'post_status' => 'inherit', + 'post_type' => 'attachment', 'post_mime_type' => 'image', - 'order' => $atts['order'], - 'orderby' => $atts['orderby'], - 'fields' => 'ids', + 'order' => $atts['order'], + 'orderby' => $atts['orderby'], + 'fields' => 'ids', ) ); } else { $attachments = get_children( array( - 'post_parent' => $id, - 'post_status' => 'inherit', - 'post_type' => 'attachment', + 'post_parent' => $id, + 'post_status' => 'inherit', + 'post_type' => 'attachment', 'post_mime_type' => 'image', - 'order' => $atts['order'], - 'orderby' => $atts['orderby'], - 'fields' => 'ids', + 'order' => $atts['order'], + 'orderby' => $atts['orderby'], + 'fields' => 'ids', ) ); } @@ -85,9 +100,17 @@ public function shortcode( $attr ) { continue; } + $href = null; + if ( ! empty( $atts['link'] ) && 'file' === $atts['link'] ) { + $href = $url; + } elseif ( ! empty( $atts['link'] ) && 'post' === $atts['link'] ) { + $href = get_attachment_link( $attachment_id ); + } + $urls[] = array( - 'url' => $url, - 'width' => $width, + 'href' => $href, + 'url' => $url, + 'width' => $width, 'height' => $height, ); } @@ -97,6 +120,12 @@ public function shortcode( $attr ) { ) ); } + /** + * Render. + * + * @param array $args Args. + * @return string Rendered. + */ public function render( $args ) { $this->did_convert_elements = true; @@ -109,24 +138,36 @@ public function render( $args ) { } $images = array(); - foreach ( $args['images'] as $image ) { - $images[] = AMP_HTML_Utils::build_tag( + foreach ( $args['images'] as $props ) { + $image = AMP_HTML_Utils::build_tag( 'amp-img', array( - 'src' => $image['url'], - 'width' => $image['width'], - 'height' => $image['height'], + 'src' => $props['url'], + 'width' => $props['width'], + 'height' => $props['height'], 'layout' => 'responsive', ) ); + + if ( ! empty( $props['href'] ) ) { + $image = AMP_HTML_Utils::build_tag( + 'a', + array( + 'href' => $props['href'], + ), + $image + ); + } + + $images[] = $image; } return AMP_HTML_Utils::build_tag( 'amp-carousel', array( - 'width' => $this->args['width'], + 'width' => $this->args['width'], 'height' => $this->args['height'], - 'type' => 'slides', + 'type' => 'slides', 'layout' => 'responsive', ), implode( PHP_EOL, $images )