General: remove backwards compatibility code now that Jetpack requires WP 5.9 #24086

jeherve · 2022-04-26T14:18:53Z

Changes proposed in this Pull Request:

General: remove backwards compatibility code for user queries
Update PHPUnit setup to remove WP < 5.9 oddities (see previous work on this in Fix Jetpack tests after WordPress backported polyfill stuff to 5.7 #21157 and jetpack: Fix tests against WordPress master #21175)

Note: we will not be able to merge this until #24083 is merged.

Jetpack product discussion

Primary issue: WordPress 6.0 compatibility #24082

Does this pull request change what data or activity we track or use?

No

Testing instructions:

Is CI happy?

See #22559

See #21175, #21157 Primary issue: #24082

matticbot · 2022-04-26T14:19:00Z

Caution: This PR has changes that must be merged to WordPress.com
Hello jeherve! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D79521-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

github-actions · 2022-04-26T14:20:09Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: June 7, 2022.
Scheduled code freeze: May 31, 2022.

anomiex

I started my own PR to do the same thing. 😀 Since you got yours first, I'll just list the extra stuff I found:

The code trying to load core's speed trap listener needs removal.

jetpack/docs/examples/bootstrap.php

Lines 66 to 69 in cde8ba3

    
           // Using the Speed Trap Listener provided by WordPress Core testing suite to expose 
        
           // slowest running tests. See the configuration in phpunit.xml.dist. 
        
           require $_tests_dir . '/includes/listener-loader.php';

Docs in monorepo.md need updating too.

jetpack/docs/monorepo.md

Lines 237 to 238 in cde8ba3

    
           <!-- @todo: Update this once we drop support for WP 5.8. --> 
        
           Note that the state of WordPress's own PHPUnit integration is currently in flux. For WordPress 5.8 and earlier you need to both use `yoast/phpunit-polyfills` to supply polyfills and need to run with PHPUnit < 8.0 (even on PHP 8, where monkey-patching is required), while for 5.9 you can use `yoast/phpunit-polyfills` normally. Your best bet for the moment is to copy what Jetpack is doing; once the situation has stabilized, we'll update this documentation and [the example bootstrap.php](./examples/bootstrap.php).

Since Gutenberg >8.0 was included in WP 5.6, we can probably take care of this:

jetpack/projects/plugins/jetpack/extensions/blocks/button/edit.js

Line 46 in cde8ba3

// TODO: Remove `replace` once minimum Gutenberg version is 8.0 (to fully support `disableLineBreaks`)
This happened back in Update wordpress monorepo #19328 and WP 5.8, we can update that and remove the referenced file too.

jetpack/projects/plugins/jetpack/extensions/blocks/story/player/modal/frame.js

Line 7 in cde8ba3

// TODO: replace with `import { useMergeRefs } from '@wordpress/compose';` when package is upgraded to ^3.24.4
Gutenberg >= 7.2 seems to have happened in WP 5.5.

jetpack/projects/plugins/jetpack/extensions/shared/jetpack-plugin-sidebar.scss

Line 22 in cde8ba3

.components-icon-button, // Gutenberg < 7.2.0. @TODO: Remove once we drop support

PHP 5.2 was dropped in WP 5.2, so all this can be reduced to basically line 223 with a variable rename.

jetpack/projects/plugins/jetpack/modules/shortcodes/slideshow.php

Lines 221 to 251 in cde8ba3

    
           		if ( defined( 'JSON_HEX_AMP' ) ) { 
        
           			// This is nice to have, but not strictly necessary since we use _wp_specialchars() below. 
        
           			$gallery = wp_json_encode( $attr['gallery'], JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT ); // phpcs:ignore PHPCompatibility 
        
           		} else { 
        
           			$gallery = wp_json_encode( $attr['gallery'] ); 
        
           		} 
        
           		$output .= '<p class="jetpack-slideshow-noscript robots-nocontent">' . esc_html__( 'This slideshow requires JavaScript.', 'jetpack' ) . '</p>'; 
        
           		/* 
        
           		 * The input to json_encode() above can contain '&quot;'. 
        
           		 * 
        
           		 * For calls to json_encode() lacking the JSON_HEX_AMP option, 
        
           		 * that '&quot;' is left unaltered.  Running '&quot;' through esc_attr() 
        
           		 * also leaves it unaltered since esc_attr() does not double-encode. 
        
           		 * 
        
           		 * This means we end up with an attribute like 
        
           		 * `data-gallery="{&quot;foo&quot;:&quot;&quot;&quot;}"`, 
        
           		 * which is interpreted by the browser as `{"foo":"""}`, 
        
           		 * which cannot be JSON decoded. 
        
           		 * 
        
           		 * The preferred workaround is to include the JSON_HEX_AMP (and friends) 
        
           		 * options, but these are not available until 5.3.0. 
        
           		 * Alternatively, we can use _wp_specialchars( , , , true ) instead of 
        
           		 * esc_attr(), which will double-encode. 
        
           		 * 
        
           		 * Since we can't rely on JSON_HEX_AMP, we do both. 
        
           		 * 
        
           		 * @todo Update when minimum is PHP 5.3+ 
        
           		 */ 
        
           		$gallery_attributes = _wp_specialchars( wp_check_invalid_utf8( $gallery ), ENT_QUOTES, false, true );

If you want I can push these changes to your branch.

docs/examples/bootstrap.php

kraftbj · 2022-04-26T21:40:16Z

Adding Cherry-Pick label for 10.9.1. Do not merge into the 10.9 branch until after 10.9.0 is stable.

Co-authored-by: Brad Jorsch <[email protected]>

…-rm-old

See #15595

See #14327

See 99fffd8

jeherve · 2022-04-27T15:06:07Z

@anomiex Thanks for the super detailed list! I took a stab at it. I have yet to test 742416b with some content that includes quotation marks in French (will do that in a bit), but I think this should be ready for a first review.

…-rm-old

anomiex

LGTM as far as it goes. The linting failure should be fixed by #24141, and of course the PHP 8 previous-WP test needs #24083 to make "previous" be 5.9.

anomiex · 2022-04-28T14:51:17Z

projects/plugins/jetpack/modules/shortcodes/slideshow.php

@@ -222,8 +222,13 @@ public function slideshow_js( $attr ) {
 		 * Checking for JSON_HEX_AMP and friends here allows us to get rid of
 		 * '&quot;', that can sometimes be included in the JSON input in some languages like French.
 		 */
-		$gallery_attributes = wp_check_invalid_utf8(
-			wp_json_encode( $attr['gallery'], JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT )
+		$gallery_attributes = _wp_specialchars(


I suspect esc_attr() might have been enough now. But this works too.

…-rm-old

github-actions · 2022-05-03T17:19:03Z

Great news! One last step: head over to your WordPress.com diff, D79521-code, and deploy it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

jeherve · 2022-05-03T17:47:26Z

r244865-wpcom

…Jetpack requires WP 5.9 (#24086) * General: remove backwards compatibility code for user queries See #22559 * Update PHPUnit setup to remove WP < 5.9 oddities See #21175, #21157 Primary issue: #24082 * Update docs/examples/bootstrap.php Co-authored-by: Brad Jorsch <[email protected]> * Remove speed trap listener loder * Update doc to remove mention of temporary PHPUnit situation * Remove temporary line break workaround See #15595 * Remove custom function in favor of the one that ships with core * Remove old workaround for the Jetpack sidebar plugin icon See #14327 * Remove PHP 5.2 workaround See 99fffd8 * Add back necessary _wp_specialchars wrapping Co-authored-by: Brad Jorsch <[email protected]> Co-authored-by: Brandon Kraft <[email protected]>

jeherve · 2022-05-19T10:38:46Z

Cherry-picked to jetpack/branch-10.9 in 04b8ad7

jeherve added 2 commits April 26, 2022 15:11

General: remove backwards compatibility code for user queries

869efe6

See #22559

Update PHPUnit setup to remove WP < 5.9 oddities

cde8ba3

See #21175, #21157 Primary issue: #24082

jeherve added this to the jetpack/10.9.1 milestone Apr 26, 2022

jeherve added General Unit Tests [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] In Progress [Type] Janitorial labels Apr 26, 2022

jeherve self-assigned this Apr 26, 2022

matticbot added the Touches WP.com Files label Apr 26, 2022

jeherve mentioned this pull request Apr 26, 2022

WordPress 6.0 compatibility #24082

Closed

6 tasks

jeherve added the [Status] Blocked / Hold label Apr 26, 2022

github-actions bot added Docs [Feature] Extra Sidebar Widgets [Feature] Widget Visibility [Feature] WPCOM API labels Apr 26, 2022

anomiex reviewed Apr 26, 2022

View reviewed changes

docs/examples/bootstrap.php Show resolved Hide resolved

kraftbj added the [Status] Needs Cherry-Pick label Apr 26, 2022

jeherve and others added 8 commits April 27, 2022 16:17

Update docs/examples/bootstrap.php

5dbc535

Co-authored-by: Brad Jorsch <[email protected]>

Merge remote-tracking branch 'origin/master' into update/wp-compat-60…

29695e9

…-rm-old

Remove speed trap listener loder

c803041

Update doc to remove mention of temporary PHPUnit situation

350438c

Remove temporary line break workaround

577b9cb

See #15595

Remove custom function in favor of the one that ships with core

a14135f

Remove old workaround for the Jetpack sidebar plugin icon

1b9c9a6

See #14327

Remove PHP 5.2 workaround

742416b

See 99fffd8

github-actions bot added [Block] Button [Block] Story [Feature] Shortcodes / Embeds labels Apr 27, 2022

jeherve added 2 commits April 28, 2022 11:36

Add back necessary _wp_specialchars wrapping

bddaced

Merge remote-tracking branch 'origin/master' into update/wp-compat-60…

235fa57

…-rm-old

anomiex approved these changes Apr 28, 2022

View reviewed changes

Merge remote-tracking branch 'origin/master' into update/wp-compat-60…

0982fe5

…-rm-old

jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress [Status] Blocked / Hold labels May 3, 2022

Merge branch 'master' into update/wp-compat-60-rm-old

3202bbc

jeherve merged commit b98c640 into master May 3, 2022

jeherve deleted the update/wp-compat-60-rm-old branch May 3, 2022 17:17

jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label May 3, 2022

jeherve removed the [Status] Needs Cherry-Pick label May 19, 2022

jeherve mentioned this pull request Dec 17, 2024

Properly escape attr_esc( json_encode( $data ) ) #1170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General: remove backwards compatibility code now that Jetpack requires WP 5.9 #24086

General: remove backwards compatibility code now that Jetpack requires WP 5.9 #24086

jeherve commented Apr 26, 2022 •

edited

Loading

matticbot commented Apr 26, 2022

github-actions bot commented Apr 26, 2022 •

edited

Loading

anomiex left a comment

kraftbj commented Apr 26, 2022

jeherve commented Apr 27, 2022

anomiex left a comment

anomiex Apr 28, 2022

github-actions bot commented May 3, 2022

jeherve commented May 3, 2022

jeherve commented May 19, 2022


	// Using the Speed Trap Listener provided by WordPress Core testing suite to expose
	// slowest running tests. See the configuration in phpunit.xml.dist.
	require $_tests_dir . '/includes/listener-loader.php';

	if ( defined( 'JSON_HEX_AMP' ) ) {
	// This is nice to have, but not strictly necessary since we use _wp_specialchars() below.
	$gallery = wp_json_encode( $attr['gallery'], JSON_HEX_TAG \| JSON_HEX_AMP \| JSON_HEX_APOS \| JSON_HEX_QUOT ); // phpcs:ignore PHPCompatibility
	} else {
	$gallery = wp_json_encode( $attr['gallery'] );
	}

	$output .= '<p class="jetpack-slideshow-noscript robots-nocontent">' . esc_html__( 'This slideshow requires JavaScript.', 'jetpack' ) . '</p>';

	/*
	* The input to json_encode() above can contain '"'.
	*
	* For calls to json_encode() lacking the JSON_HEX_AMP option,
	* that '"' is left unaltered. Running '"' through esc_attr()
	* also leaves it unaltered since esc_attr() does not double-encode.
	*
	* This means we end up with an attribute like
	* `data-gallery="{"foo":"""}"`,
	* which is interpreted by the browser as `{"foo":"""}`,
	* which cannot be JSON decoded.
	*
	* The preferred workaround is to include the JSON_HEX_AMP (and friends)
	* options, but these are not available until 5.3.0.
	* Alternatively, we can use _wp_specialchars( , , , true ) instead of
	* esc_attr(), which will double-encode.
	*
	* Since we can't rely on JSON_HEX_AMP, we do both.
	*
	* @todo Update when minimum is PHP 5.3+
	*/
	$gallery_attributes = _wp_specialchars( wp_check_invalid_utf8( $gallery ), ENT_QUOTES, false, true );

General: remove backwards compatibility code now that Jetpack requires WP 5.9 #24086

General: remove backwards compatibility code now that Jetpack requires WP 5.9 #24086

Conversation

jeherve commented Apr 26, 2022 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

matticbot commented Apr 26, 2022

github-actions bot commented Apr 26, 2022 • edited Loading

anomiex left a comment

Choose a reason for hiding this comment

kraftbj commented Apr 26, 2022

jeherve commented Apr 27, 2022

anomiex left a comment

Choose a reason for hiding this comment

anomiex Apr 28, 2022

Choose a reason for hiding this comment

github-actions bot commented May 3, 2022

jeherve commented May 3, 2022

jeherve commented May 19, 2022

jeherve commented Apr 26, 2022 •

edited

Loading

github-actions bot commented Apr 26, 2022 •

edited

Loading