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.

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

Adding Post Comments Form block to post causes validation error for comment-reply enqueued script #6231

Closed
westonruter opened this issue May 13, 2021 · 3 comments · Fixed by #6579 or #6546
Assignees
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. Integration: Gutenberg
Milestone

Comments

@westonruter
Copy link
Member

Bug Description

As part of Full Site Editing, Gutenberg has a “Post Comments Form” block:

image

This renders the standard comments form inside the post content:

image

When viewing such a post on the frontend, a validation error is reported:

image

Normally the comment-reply form is dequeued when the singular template is loaded:

add_action(
'wp_enqueue_scripts',
static function() {
wp_dequeue_script( 'comment-reply' ); // Handled largely by AMP_Comments_Sanitizer and *reply* methods in this class.
}
);

This doesn't work for the block, however, because the script is enqueued as part of the render callback.

This is the second block we've seen in core to enqueue a script, the first being the File block which we added support for in #6112. We may need to take a similar approach where we dequeue the script just-in-time at wp_print_scripts /wp_print_footer_scripts. For example:

--- a/includes/class-amp-theme-support.php
+++ b/includes/class-amp-theme-support.php
@@ -898,12 +898,12 @@ static function() {
 			},
 			0
 		);
-		add_action(
-			'wp_enqueue_scripts',
-			static function() {
-				wp_dequeue_script( 'comment-reply' ); // Handled largely by AMP_Comments_Sanitizer and *reply* methods in this class.
-			}
-		);
+
+		$dequeue_comment_reply = static function() {
+			wp_dequeue_script( 'comment-reply' ); // Handled largely by AMP_Comments_Sanitizer and *reply* methods in this class.
+		};
+		add_action( 'wp_print_scripts', $dequeue_comment_reply, 0 );
+		add_action( 'wp_print_footer_scripts', $dequeue_comment_reply, 0 );
 	}
 
 	/**

Note also that the source of this script is erroneously being reported as Twenty Twenty-One when it should be Core.

Expected Behaviour

Steps to reproduce

  1. Activate Gutenberg.
  2. Add the Post Comments Form to the content
  3. Validate the AMP page.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@delawski
Copy link
Collaborator

delawski commented Dec 1, 2021

QA Passed

✅ After activating the TT1 Blocks theme and adding a Post Comments Form to a post, I got no "Invalid script" validation issue:

Screenshot 2021-12-01 at 17 25 18

Note that there was no Post Comments Form available in the regular Twenty Twenty-One theme. Following the instruction on make.wordpress.org, I went ahead and installed the TT1 Blocks theme.

Tested on AMP Version 2.2.0-alpha-20211123T020732Z-5405daa

@delawski
Copy link
Collaborator

delawski commented Dec 1, 2021

@westonruter While QA'ing this issue, I've noticed a potential new issue.

The TT1 Blocks theme replaces the default theme header with a block-based template part. The template part uses a Navigation block which seems to be enqueuing yet another script, gutenberg/build/block-library/blocks/navigation/view.min.js. Similarly to comment-reply.js, it causes an AMP validation error:

Screenshot 2021-12-01 at 17 35 53

Should I create a new bug report for that or will we take care of FSE and TT1 Blocks theme later (as I think there might be more blocks causing similar errors)?

@delawski delawski self-assigned this Dec 1, 2021
@westonruter
Copy link
Member Author

@delawski I think this will be addressed by #6319 for #6649. So I think this is captured already and it's being tracked for v2.3.

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. Integration: Gutenberg
Projects
None yet
3 participants