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

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

Open
westonruter opened this issue Nov 8, 2023 · 1 comment
Open

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

westonruter opened this issue Nov 8, 2023 · 1 comment
Assignees
@thelovekesh
Labels
Bug Something isn't working P0 High priority
Projects
Ongoing
Milestone
v2.6

Comments

@westonruter
Copy link
Member

westonruter commented Nov 8, 2023 •
edited

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 Remove deprecated controls from editor sidebar and fix AMP Lightbox and AMP Carousel in Gallery block #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 

[
	{
		"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 westonruter added Bug Something isn't working P0 High priority labels Nov 8, 2023
@westonruter westonruter added this to the v2.5.1 milestone Nov 8, 2023
@westonruter westonruter added this to Backlog in Ongoing via automation Nov 8, 2023
@westonruter westonruter moved this from Backlog to To Do in Ongoing Nov 8, 2023
@westonruter
Copy link
Member Author

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.

@westonruter westonruter mentioned this issue Nov 30, 2023
Closed
@westonruter westonruter modified the milestones: v2.5.1, v2.5.2 Dec 18, 2023
@westonruter westonruter modified the milestones: v2.5.2, v2.5.3 Jan 8, 2024
@westonruter westonruter modified the milestones: v2.5.3, v2.5.4 Mar 14, 2024
@westonruter westonruter modified the milestones: v2.5.4, v2.6 May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thelovekesh thelovekesh

Labels
Bug Something isn't working P0 High priority
Projects
Ongoing
  
To Do
Milestone
v2.6
Development

No branches or pull requests
2 participants
@westonruter @thelovekesh