Regression: Fix duotone support to be compatible with enhanced pagination

[cbravobernal]

Reference: ​Gutenberg PR 55415

This fix makes the duotone compatible with the enhanced pagination by making sure that the CSS is always on the page, even when the posts have no featured image. It also prevents the duotone from interfering with other blocks using wp_unique_id.

With the current implementation of enhanced pagination, the CSS corresponding to each block is still not detected. Therefore, for the enhanced pagination to work correctly, the CSS of the blocks present in the Post Template must be stable on all pages.

The featured image block can contain a duotone, but the duotone CSS is only inlined when the block is not empty. As posts can have or not have a featured image, if all posts on one page do not have a featured image, the duotone CSS will not load, and when you paginate, the CSS will not be applied to the posts that do have a featured image.

In addition, some duotone settings use the wp_unique_id, and as it is a conditional use, this can break the stability of the rest of the blocks that use wp_unique_id causing another series of problems like those described in ​https://github.com/WordPress/gutenberg/issues/55230).

Trac ticket: https://core.trac.wordpress.org/ticket/59694

This PR backports ​#55415 that fixes an issue with duotone and enhanced pagination. All the context is inside Gutenberg PR.

I'm not finding an easy way to test that in render_duotone_support() function with an empty $block_content, we are not returning early like we did before.

I tried checking that get_all_global_style_block_names() is called, but, as is a private static function, I couldn't make it work with getMockBuilder or ReflectionMethod

Any help would be kindly appreciated.

Hi there!

Pinging @tellthemachines for a review 😅
Thanks a lot!!

[hellofromTonya]

Thanks for opening the ticket to merge the changes from Gutenberg into Core.

Marking it for commit to Core's trunk.

What about backporting the change to the 6.4 branch?

@mikachan @cbravobernal Is this bugfix resolving a regression or break / error introduced during the 6.4 cycle?

​See Make/Core post "WordPress 6.4 Release Candidate Phase":

Regressions: bugs that have been introduced during the WordPress 6.4 development cycle, either to existing or new features.

Exploring the fix:

• The change removes a conditional introduced in [56226] / #58673, i.e. during 6.3 cycle.
• [56101] first introduced the WP_Duotone class during the 6.3 cycle.
• The ​issue in Gutenberg seems to be a bugfix to further address issues introduced in 6.3. It was opened 2 weeks ago and punted to 6.4.1.

In reviewing, it does not seem to fit within the RC guidelines. Awaiting more information before punting from 6.4.0 milestone.

[hellofromTonya]

This change is a partial fix for ​the Gutenberg issue 55230. The other half of the fix is in ticket #59681. Both tickets should be considered together for whether they should or shouldn't be backported to the 6.4 branch, as together they solve the issue with enhanced pagination in the Query block.

[hellofromTonya]

Sharing more information on whether this should or shouldn't be backported to the 6.4-branch.

As @cbravobernal ​noted in Make/Core slack core channel, the bug did happen in 6.4, as the enhanced pagination is being introduced in 6.4.

the layout support was working fine in 6.3. This is a compatibility fix with the new enhanced pagination of the Query block, which is being introduced in 6.4...

..this fixes an edge case only affecting sites using the new enhanced pagination in conjunction with featured images that are styled with a doutone

Patch Report

Patch tested: ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/5540.diff

How to read this report:

🐞 <= Bug occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

Environment

• OS: macOS 14.0
• Web Server: Nginx
• macOS 14.0
• PHP 8.2.11
• 6.4-RC1-56475-src
• Firefox 118.0.2

Steps to Reproduce

1. With WordPress 6.4 installed
2. Create the three posts with featured image, and another three without featured image.
3. On the site editor, in home page, TT4, add pagination to the Query Loop, set it to inherit the template settings, activate "Enhanced Pagination feature)
4. Add duotone to the feature image inside the Query Loop.
5. Make sure the three posts shown in the home page don't have featured image. Use pagination.
6. 🐞 Check that on page two, duotone filter is not applied for next posts.

Steps to Test Patch

1. 🛠 Apply ​the patch.
2. Refresh the home page.
3. ✅ Check that the duotone filter is applied after navigating to the second page.
4. ✅ Check that the navigation happened without a page reload.

Test Results

• ✅ Issue is reproducible.
• ✅ Issue resolved with patch.

Screenshots

Before patch:

After patch:

[hellofromTonya]

In discussions and reviews, this is fixing a 6.4 regression introduced with the new enhanced pagination feature. Thus it is okay to be fixed in the 6.4 RC cycle.

[jorbin]

In general, this looks good. I left one comment but I don't think it needs to block merge.

I do think there is further opportunity to improve the automated tests. For example, all the tests right now assume that the DuoTone should be added. This feels like a 6.5 improvement thought.

For commit prep, I'll be pushing a few commits to this PR to address the failing PHP 8.3 test, review notes from @aaronjorbin, resetting the reflected property's visibility in the test, and a few other minor formatting things.

[hellofromTonya]

Prepping the commit for trunk now.

[hellofromTonya]

In 56991:

Editor: Fix render_duotone_support() to be compatible with enhanced pagination.

Some blocks do not have content. For duotone support, blocks without content still need to run through the render_duotone_support() to render their duotone CSS.

This fix makes the duotone compatible with the enhanced pagination (introduced in 6.4.0) by making sure that the CSS is always on the page, even when the posts have no featured image. It also prevents the duotone from interfering with other blocks using wp_unique_id().

References:

• ​Gutenberg PR 55415

Follow-up to [56226].

Props cbravobernal, luisherranz, hellofromTonya, isabel_brison, jorbin.
Fixes #59694.

Committed via https://core.trac.wordpress.org/changeset/56991.

[hellofromTonya]

Reopening for 2nd committer sign-off to backport [56991] to the 6.4 branch.

@costdev found the test in r56991 did not properly handle the private static property:

• It set the object using WP_Block 🤦‍♀️
• It needs to also reset to the original value when done.

As this property is static and the class is also static by design, an object cannot be passed when setting the value. Instead, ReflectionClass is needed, which exposes a getter (getStaticPropertyValue()) and setter (setStaticPropertyValue()) for static properties.

Trac ticket: https://core.trac.wordpress.org/ticket/59694

[hellofromTonya]

Resetting the commit keyword as @costdev found the test in [56991] was incorrectly handling the static property. ​https://github.com/WordPress/wordpress-develop/pull/5561 is a follow-up to resolve the reflection needs in the test.

@costdev found the test in r56991 did not properly handle the private static property:

• It set the object using WP_Block 🤦‍♀️
• It needs to also reset to the original value when done.

This PR fixes both of these issues.

It also addresses handling issues.

It uses ReflectionClass which works well with static classes, i.e. a class that only contains static properties and methods. It then uses ReflectionClass::getProperty() to get an instance of ReflectionClass, which it then uses for changing the property's visibility, getting its value, and setting its value.

It also adds an inline comment to explain why an instance of WP_Duotone is needed. IMO having an instance is confusing.

Why? WP_Duotone is a static class by design (as noted above), meaning it's not intended to be an object. But as of PHP 8.3, not passing an instance to ReflectionProperty::setValue() is deprecated. Therefore, an instance is needed to run in all Core supported PHP versions.

Trac ticket: https://core.trac.wordpress.org/ticket/59694

Follow-up: This approach works awesome in PHP 7.4 and up, but it does not with < PHP 7.4 as shown by the failing tests for PHP 7.0, 7.1, and 7.3. Closing this PR in favor of ​https://github.com/WordPress/wordpress-develop/pull/5561.

[costdev]

@hellofromTonya ​PR 5559 looks good for commit to trunk and the PR and [56991] look good for merge to the 6.4 branch.

Thanks for working on these everyone!

[hellofromTonya]

Preparing the commits now.

[hellofromTonya]

In 56996:

Tests: Fix static property handling in r56991.

Fixes static property handling for WP_Duotone::$block_css_declarations in the Tests_Block_Supports_Duotone::test_css_declarations_are_generated_even_with_empty_block_content():

• Fixes ReflectionProperty::setValue() to use an instance of WP_Duotone.
• Adds an inline comment to explain why a static class (i.e. a class that is not intended to be an object by design as it only contains static properties and methods) needs an instance, i.e. needed for PHP 8.3 and higher.
• Resets the static property's value to its original value, i.e. before the test started.

Follow-up to [56991].

Props costdev.
See #59694.

[hellofromTonya]

In 56997:

Editor: Fix render_duotone_support() to be compatible with enhanced pagination.

Some blocks do not have content. For duotone support, blocks without content still need to run through the render_duotone_support() to render their duotone CSS.

This fix makes the duotone compatible with the enhanced pagination (introduced in 6.4.0) by making sure that the CSS is always on the page, even when the posts have no featured image. It also prevents the duotone from interfering with other blocks using wp_unique_id().

References:

• ​Gutenberg PR 55415

Follow-up to [56226].

Props cbravobernal, luisherranz, hellofromTonya, isabel_brison, jorbin.
Reviewed by costdev.
Merges [56991] and [56996] to the 6.4 branch.
Fixes #59694.

Committed into trunk via https://core.trac.wordpress.org/changeset/56996 and to 6.4 branch via https://core.trac.wordpress.org/changeset/56997.