ampproject / amp-wp

Enable AMP on your WordPress site, the WordPress way.
https://wordpress.org/plugins/amp/
GNU General Public License v2.0
1.79k stars 384 forks source link

Lightbox images ("Expand on click") in WordPress 6.4 is broken on AMP pages #7676

Open westonruter opened 1 year ago

westonruter commented 1 year ago

Bug Description

WordPress 6.4 introduces lightboxing for images ("Expand on click"). There are two new issues here:

  1. This is redundant now with our existing "Add lightbox effect" in the AMP Settings panel. We can now deprecate the AMP lightbox setting (and the AMP Settings panel entirely?) following up on https://github.com/ampproject/amp-wp/pull/6833. Block deprecation routines should automatically convert the AMP lightbox block attribute to the new core lightbox block attribute. This would only be applicable to Wordpress 6.4+. Note: There is also a site-wide option to enable lightboxing by default under Styles » Blocks » Images.
  2. Core's lightbox functionality is breaking AMP validation. A new sanitizer or embed handler is needed to automatically convert lightboxed image blocks over to use the AMP lightbox functionality. So while the dedicated AMP UI for lightboxing should be removed in WP 6.4+, the underlying AMP lightbox sanitizer/embed code would remain and be reused now for implementing core's lightbox handling. Nevertheless, some changes may be needed to make the AMP implementation more in-line with what core is doing.
Validation data for 4 validation errors

```json [ { "code": "ATTR_REQUIRED_BUT_MISSING", "attributes": [ "src" ], "spec_name": "amp-img", "node_name": "img", "parent_name": "figure", "type": "html_element_error", "node_attributes": { "decoding": "async", "data-wp-bind--src": "selectors.core.image.enlargedImgSrc", "data-wp-style--object-fit": "selectors.core.image.lightboxObjectFit", "src": "", "alt": "", "class": "wp-image-77" }, "node_type": "ELEMENT", "sources": [ { "hook": "the_content", "filter": true, "post_id": 76, "post_type": "post", "sources": [ { "type": "core", "name": "wp-includes", "file": "class-wp-embed.php", "line": 62, "function": "WP_Embed::run_shortcode" }, { "type": "core", "name": "wp-includes", "file": "class-wp-embed.php", "line": 442, "function": "WP_Embed::autoembed" }, { "type": "core", "name": "wp-includes", "file": "blocks.php", "line": 1517, "function": "do_blocks" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 37, "function": "wptexturize" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 446, "function": "wpautop" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 824, "function": "shortcode_unautop" }, { "type": "core", "name": "wp-includes", "file": "post-template.php", "line": 1712, "function": "prepend_attachment" }, { "type": "core", "name": "wp-includes", "file": "https-migration.php", "line": 51, "function": "wp_replace_insecure_home_url" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 5697, "function": "capital_P_dangit" }, { "type": "core", "name": "wp-includes", "file": "shortcodes.php", "line": 243, "function": "do_shortcode" }, { "type": "core", "name": "wp-includes", "file": "media.php", "line": 1821, "function": "wp_filter_content_tags" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 3501, "function": "convert_smilies" }, { "type": "plugin", "name": "amp", "file": "includes/validation/class-amp-validation-manager.php", "line": 1581, "function": "AMP_Validation_Manager::decorate_filter_source" } ] }, { "block_name": "core/image", "post_id": 76, "block_content_index": 0, "block_attrs": { "lightbox": { "enabled": true }, "id": 77, "sizeSlug": "large", "linkDestination": "none" }, "type": "core", "name": "wp-includes", "file": "blocks/image.php", "line": 18, "function": "render_block_core_image" } ], "removed": true, "reviewed": false }, { "code": "ATTR_REQUIRED_BUT_MISSING", "attributes": [ "src" ], "spec_name": "amp-img", "node_name": "img", "parent_name": "figure", "type": "html_element_error", "node_attributes": { "decoding": "async", "data-wp-bind--src": "context.core.image.imageCurrentSrc", "data-wp-style--object-fit": "selectors.core.image.lightboxObjectFit", "src": "", "alt": "", "class": "wp-image-77" }, "node_type": "ELEMENT", "sources": [ { "hook": "the_content", "filter": true, "post_id": 76, "post_type": "post", "sources": [ { "type": "core", "name": "wp-includes", "file": "class-wp-embed.php", "line": 62, "function": "WP_Embed::run_shortcode" }, { "type": "core", "name": "wp-includes", "file": "class-wp-embed.php", "line": 442, "function": "WP_Embed::autoembed" }, { "type": "core", "name": "wp-includes", "file": "blocks.php", "line": 1517, "function": "do_blocks" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 37, "function": "wptexturize" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 446, "function": "wpautop" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 824, "function": "shortcode_unautop" }, { "type": "core", "name": "wp-includes", "file": "post-template.php", "line": 1712, "function": "prepend_attachment" }, { "type": "core", "name": "wp-includes", "file": "https-migration.php", "line": 51, "function": "wp_replace_insecure_home_url" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 5697, "function": "capital_P_dangit" }, { "type": "core", "name": "wp-includes", "file": "shortcodes.php", "line": 243, "function": "do_shortcode" }, { "type": "core", "name": "wp-includes", "file": "media.php", "line": 1821, "function": "wp_filter_content_tags" }, { "type": "core", "name": "wp-includes", "file": "formatting.php", "line": 3501, "function": "convert_smilies" }, { "type": "plugin", "name": "amp", "file": "includes/validation/class-amp-validation-manager.php", "line": 1581, "function": "AMP_Validation_Manager::decorate_filter_source" } ] }, { "block_name": "core/image", "post_id": 76, "block_content_index": 0, "block_attrs": { "lightbox": { "enabled": true }, "id": 77, "sizeSlug": "large", "linkDestination": "none" }, "type": "core", "name": "wp-includes", "file": "blocks/image.php", "line": 18, "function": "render_block_core_image" } ], "removed": true, "reviewed": false }, { "node_name": "script", "parent_name": "body", "code": "DISALLOWED_TAG", "type": "js_error", "node_attributes": { "src": "http://localhost:10003/wp-includes/js/dist/interactivity.min.js?ver=__normalized__", "id": "wp-interactivity-js", "defer": "defer", "data-wp-strategy": "defer" }, "node_type": "ELEMENT", "sources": [ { "type": "core", "name": "wp-includes", "file": "blocks/image.php", "line": 358, "function": "register_block_core_image", "hook": "init", "priority": 10, "dependency_type": "script", "handle": "wp-block-image-view", "dependency_handle": "wp-interactivity" }, { "type": "core", "name": "wp-includes", "file": "blocks.php", "line": 1517, "function": "do_blocks", "hook": "the_content", "priority": 9, "dependency_type": "script", "handle": "wp-block-image-view", "dependency_handle": "wp-interactivity" }, { "type": "core", "name": "wp-includes", "file": "script-loader.php", "line": 659, "function": "wp_default_packages", "hook": "wp_default_scripts", "priority": 10, "dependency_type": "script", "handle": "wp-interactivity" }, { "type": "core", "name": "wp-includes", "file": "blocks/file.php", "line": 92, "function": "register_block_core_file", "hook": "init", "priority": 10, "dependency_type": "script", "handle": "wp-interactivity" }, { "type": "core", "name": "wp-includes", "file": "script-loader.php", "line": 2239, "function": "wp_print_footer_scripts", "hook": "wp_footer", "priority": 20 }, { "type": "core", "name": "wp-includes", "file": "script-loader.php", "line": 2229, "function": "_wp_footer_scripts", "hook": "wp_print_footer_scripts", "priority": 10 } ], "removed": true, "reviewed": false }, { "node_name": "script", "parent_name": "body", "code": "DISALLOWED_TAG", "type": "js_error", "node_attributes": { "src": "http://localhost:10003/wp-includes/blocks/image/view.min.js?ver=__normalized__", "id": "wp-block-image-view-js", "defer": "defer", "data-wp-strategy": "defer" }, "node_type": "ELEMENT", "sources": [ { "type": "core", "name": "wp-includes", "file": "blocks/image.php", "line": 358, "function": "register_block_core_image", "hook": "init", "priority": 10, "dependency_type": "script", "handle": "wp-block-image-view" }, { "type": "core", "name": "wp-includes", "file": "blocks.php", "line": 1517, "function": "do_blocks", "hook": "the_content", "priority": 9, "dependency_type": "script", "handle": "wp-block-image-view" }, { "type": "core", "name": "wp-includes", "file": "script-loader.php", "line": 2239, "function": "wp_print_footer_scripts", "hook": "wp_footer", "priority": 20 }, { "type": "core", "name": "wp-includes", "file": "script-loader.php", "line": 2229, "function": "_wp_footer_scripts", "hook": "wp_print_footer_scripts", "priority": 10 } ], "removed": true, "reviewed": false } ] ```

Expected Behaviour

Core image lightboxes should work on AMP pages without validation errors. There should not be redundant UI elements for lightboxing.

Screenshots

image

image

PHP Version

8.1

Plugin Version

2.5.0

AMP plugin template mode

Standard, Transitional, Reader

WordPress Version

6.4

Site Health

No response

Gutenberg Version

No response

OS(s) Affected

No response

Browser(s) Affected

No response

Device(s) Affected

No response

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

westonruter commented 1 year ago

For reference, this is what the non-AMP lightbox functionality looks like:

Screen recording 2023-11-09 09.42.19.webm

I don't think it's too important to make sure that the AMP version looks and behaves exactly the same. That would be a bonus.