HTML API: Backport updates from Core

[dmsnell]

This is a backport PR to bring updates from changes in WordPress/WordPress-develop at 54a09a7e into this repository into a compatibility layer.

• WIP: HTML API: Add support for H1-H6 elements in HTML Processor wordpress-develop#5535
• HTML API: avoid processing incomplete tokens wordpress-develop#5725
• HTML API: Add explicit handling or failure for all tags wordpress-develop#5762
• HTML API: Add support for list elements. wordpress-develop#5539

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/compat/wordpress-6.5/html-api/class-gutenberg-html-open-elements-6-5.php
❔ lib/compat/wordpress-6.5/html-api/class-gutenberg-html-processor-6-5.php
❔ lib/compat/wordpress-6.5/html-api/class-gutenberg-html-processor-state-6-5.php
❔ lib/compat/wordpress-6.5/html-api/class-gutenberg-html-tag-processor-6-5.php
❔ lib/load.php

[github-actions]

Flaky tests detected in f02244a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7280154863
📝 Reported issues:

• [Flaky Test] Should show Columns block only if selected blocks are between limits (1-6) #40369 in specs/editor/various/block-switcher.test.js

[dmsnell]

@SergeyBiryukov when the WPCS ignore statements were removed in https://core.trac.wordpress.org/changeset/56753 it started preventing updates in this repo to the file where it was removed. I couldn't find a Trac ticket or any code review for that change before it was merged.

@hellofromtonya @anton-vlasenko I don't like merging over failing CI steps so I would appreciate your guidance. the WPCS rules which are preventing this PR from maintaining code harmony between the projects are the same rules that forced the change that created the obstacle; this seems silly.

[anton-vlasenko]

I don't like merging over failing CI steps so I would appreciate your guidance. the WPCS rules which are preventing this PR from maintaining code harmony between the projects are the same rules that forced the change that created the obstacle; this seems silly.

I'm sorry to hear that misalignment in coding standards is still causing issues, @dmsnell. Hopefully, one day, coding standards can be fully aligned.

In my opinion , VariableAnalysis.CodeAnalysis.VariableAnalysis should be removed from Gutenberg since it is not enabled in Core. However, it is essential to analyze why it was added in the first place. Until this analysis takes place, I'd suggest adding the <exclude-pattern /> statement for that folder:
I.e. replacing

<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
    <properties>
        <property name="allowUnusedParametersBeforeUsed" value="true"/>
    </properties>
</rule>

with

<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
    <properties>
        <property name="allowUnusedParametersBeforeUsed" value="true"/>
    </properties>
    <!-- Exclude the html-api folder to prevent linter errors until coding standards are aligned. -->
    <exclude-pattern>./lib/compat/wordpress-*/html-api/*.php</exclude-pattern>
</rule>

What is your opinion on this?

[dmsnell]

What is your opinion on this?

Sounds great @anton-vlasenko - I appreciate making the adjustment to the rules, and avoiding the need to add more IGNORE statements. I'll add that change in this backport PR. For some reason I often forget that I can modify the WPCS rules in one of these backport PRs.

Thanks as always for your help.

Hopefully, one day, coding standards can be fully aligned.

We'll get there. Things have been getting better. I think in this case the problem happened because someone changed Core in a way that broke Gutenberg but that change was never run through any review process or preview in Gutenberg, so the error didn't appear until it was too late.

[ockham]

My personal preference aside, I'm afraid we can't merge this as-is: The problem is that the Dynamic Block PHP code (i.e. everything in packages/block-library/src/**/*.php) will be carried over into Core via npm package sync. But in Core, none of the back-compat Gutenberg_HTML_Tag_Processor_6_x classes exist, so it would fatal there 😕

(The same might apply to lib/block-supports/*.php, but I'm not sure about that.)

[dmsnell]

carried over into Core via npm package sync. But in Core, none of the back-compat Gutenberg_HTML_Tag_Processor_6_x classes exist, so it would fatal there 😕

is there not a way we can replace those classes during the packaging? and doesn't this same problem exist in other shared code?

I can pull these out if we need to; still trying to find a more reliable method to avoid breakages.

[ockham]

carried over into Core via npm package sync. But in Core, none of the back-compat Gutenberg_HTML_Tag_Processor_6_x classes exist, so it would fatal there 😕

is there not a way we can replace those classes during the packaging?

We have sort of the reverse mechanism in place for some function names in Dynamic Blocks' PHP (that are changed from wp_ to gutenberg_) at build time:

gutenberg/tools/webpack/blocks.js

Lines 25 to 36 in 1275d2f

	/**
	* We need to automatically rename some functions when they are called inside block files,
	* but have been declared elsewhere. This way we can call Gutenberg override functions, but
	* the block will still call the core function when updates are back ported.
	*/
	const prefixFunctions = [
	'wp_apply_colors_support',
	'wp_enqueue_block_support_styles',
	'wp_get_typography_font_size_value',
	'wp_style_engine_get_styles',
	'wp_get_global_settings',
	];

To avoid confusion, we'd need to align with that and also transform from WP_ to Gutenberg_ (rather than the other way round).

and doesn't this same problem exist in other shared code?

I think we have a recommendation somewhere that Dynamic Blocks generally use functions rather than classes (and class methods); although upon some further research, there seems to be some precedent in the guise of some Style Engine related class names which are indeed transformed:

gutenberg/tools/webpack/packages.js

Lines 40 to 46 in 1275d2f

	replaceClasses: [
	'WP_Style_Engine_CSS_Declarations',
	'WP_Style_Engine_CSS_Rules_Store',
	'WP_Style_Engine_CSS_Rule',
	'WP_Style_Engine_Processor',
	'WP_Style_Engine',
	],

...although the transformed names seem to look a bit different than the "usual" Gutenberg_*_x_y scheme:

gutenberg/tools/webpack/packages.js

Lines 57 to 64 in 1275d2f

	const classSuffix = '_Gutenberg';
	const functionPrefix = 'gutenberg_';
	content = content.toString();
	// Replace class names.
	content = content.replace(
	new RegExp( replaceClasses.join( '|' ), 'g' ),
	( match ) => `${ match }${ classSuffix }`
	);

I can pull these out if we need to; still trying to find a more reliable method to avoid breakages.

Thank you. How about we land the backport of WordPress/wordpress-develop#5535 and WordPress/wordpress-develop#5725 separately first (to unblock further work), and file a separate PR for the other changes (since they might need some more tweaking)?

---

TBH I'm still not 100% convinced we should really duplicate all classes from Core in GB's back-compat layer and update all code to use the latter, given that it'll add a ton more code that will need syncing and require that extra magic at build time, which will likely mean another list of classes tucked away in a webpack config that we'll need to keep updated.

[dmsnell]

@ockham this has been pared back to only include the changes to the existing classes.

[ockham]

LGTM!

I've compared all files to their counterparts in the https://github.com/WordPress/wordpress-develop repo. They are all exact matches, except for the different class names (as expected).