Shortcodes: fix [archiveorg] query parameter handling by zdenys · Pull Request #49195 · Automattic/jetpack

zdenys · 2026-05-27T11:39:16Z

Proposed changes

Build the [archiveorg] embed URL with add_query_arg() so autoplay, poster, and the newly-supported playlist attribute render as a proper ?key=value&key=value query string instead of being concatenated into the URL path with stray & separators.
Accept query parameters baked into the identifier (e.g. [archiveorg myitem&playlist=1] or [archiveorg myitem?playlist=1]), splitting them off the id and re-appending them with the correct separator. This keeps existing posts working — including the ?-prefix workaround that's been shared as guidance.
Add PHPUnit coverage for the four cases above.

Related product discussion/links

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

No.

Testing instructions

On a site with the Jetpack shortcodes module active, add a post containing each of the following shortcodes:
- [archiveorg sentidodelobjeto&playlist=1 width=640 height=300]
- [archiveorg sentidodelobjeto?playlist=1 width=640 height=300]
- [archiveorg id=sentidodelobjeto playlist=1]
- [archiveorg id=Wonderfu1958 autoplay=1 poster=https://archive.org/img.png]
View the rendered post and inspect each iframe's src:
- For the first three, the URL should be https://archive.org/embed/sentidodelobjeto?playlist=1 and the playlist should display on archive.org's end.
- For the fourth, the URL should be https://archive.org/embed/Wonderfu1958?autoplay=1&poster=....
Confirm that a plain [archiveorg Wonderfu1958] (no extras) still produces https://archive.org/embed/Wonderfu1958 unchanged.
Optionally run the suite: jp docker phpunit jetpack -- --filter Jetpack_Shortcodes_ArchiveOrg_Test.

Build the embed URL with add_query_arg so autoplay, poster, and the newly-supported playlist attribute render as a proper query string. Also accept query parameters baked into the identifier (either "?" or "&" separator) so existing posts continue to work.

github-actions · 2026-05-27T11:40:02Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the fix/archiveorg-shortcode-query-parameters branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack fix/archiveorg-shortcode-query-parameters

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2026-05-27T11:41:05Z

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.
✅ Add a "[Status]" label (In Progress, Needs 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 🤖

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

add_query_arg() does not urlencode values, so the poster URL is embedded verbatim in the query string.

jp-launch-control · 2026-05-27T12:13:11Z

Code Coverage Summary

Coverage changed in 1 file.

File	Coverage	Δ%	Δ Uncovered
projects/plugins/jetpack/modules/shortcodes/archiveorg.php	50/86 (58.14%)	18.41%	-8 💚

Full summary · PHP report · JS report

zdenys · 2026-05-27T12:50:25Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the fix/archiveorg-shortcode-query-parameters branch.

Done. No issues.

zdenys · 2026-05-27T13:16:52Z

To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack fix/archiveorg-shortcode-query-parameters

From the test instruction, [archiveorg sentidodelobjeto&playlist=1 width=640 height=300] didn't render correctly due to & converted to & in https://archive.org/embed/sentidodelobjeto?amp;playlist=1. Will propose a new commit shortly to address this.

When the_content encodes "&" to "&" before do_shortcode runs, the shortcode parser receives "myitem&playlist=1" and the split picks up "amp;playlist=1" as the trailing query string. Decode & back to & in the identifier before splitting, matching what the sibling jetpack_archiveorg_embed_to_shortcode() function already does.

zdenys · 2026-05-27T13:49:49Z

Tested on Simple again with 7e51465 and this time everything worked as expeted.

Copilot

Pull request overview

This PR fixes Archive.org shortcode embed URL generation so query parameters are appended as proper query strings, including newly supported playlist handling and legacy query parameters embedded in the identifier.

Changes:

Builds Archive.org embed URLs with add_query_arg().
Adds support for playlist=1 as a shortcode attribute.
Adds PHPUnit coverage for query parameters supplied via attributes and positional identifiers.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
`projects/plugins/jetpack/modules/shortcodes/archiveorg.php`	Updates Archive.org shortcode URL construction and query-parameter handling.
`projects/plugins/jetpack/tests/php/modules/shortcodes/Jetpack_Shortcodes_ArchiveOrg_Test.php`	Adds tests for autoplay/poster, playlist, and query strings embedded in identifiers.
`projects/plugins/jetpack/changelog/fix-archiveorg-shortcode-query-parameters`	Adds a Jetpack changelog entry for the shortcode fix.

kraftbj

Really nice fix. 🙌 Moving the URL construction to add_query_arg() is the right call, and the &-normalization is a thoughtful catch for the the_content-encodes-before-do_shortcode case. The PHPUnit coverage across all four scenarios is much appreciated too.

Just a few minor, non-blocking tweaks inline before merge. None are blockers and the core behavior is correct for the normal cases. I also double-checked the security side: with the hardcoded archive.org/embed/ host and the final esc_url(), there's no escape/XSS concern here, and poster is actually a touch safer than before.

kraftbj · 2026-05-29T15:44:47Z

+	// jetpack_archiveorg_embed_to_shortcode() splits on "&amp;" for the same reason.
+	$id          = str_replace( '&amp;', '&', $id );
+	$extra_query = '';
+	if ( preg_match( '/^([^?&]*)[?&](.*)$/', $id, $id_match ) ) {


Small edge case: the missing-ID guard above (if ( ! $atts['id'] )) runs before this split, so an identifier that's only a query string — e.g. [archiveorg ?playlist=1] or [archiveorg &playlist=1] — slips past it. After the split $id ends up empty and the output becomes https://archive.org/embed/?playlist=1 (an item-less embed) instead of the intended error comment. A quick '' === $id re-check right after the split would close it. Very minor since it only fires on malformed input.

Good catch — addressed in 699ae7c with a '' === $id re-check right after the split, returning the same  comment. Added a test that covers both ?playlist=1 and &playlist=1 shapes.

kraftbj · 2026-05-29T15:44:47Z

+		$url = add_query_arg( $query_args, $url );
+	}
+	if ( '' !== $extra_query ) {
+		$url .= ( str_contains( $url, '?' ) ? '&' : '?' ) . $extra_query;


Optional consolidation (fine for a follow-up): appending $extra_query raw after add_query_arg() means the two query sources can't merge or dedupe, which leaves a couple of edge cases:

Duplicate keys when a param is supplied both ways: [archiveorg myitem&playlist=1 playlist=1] → ?playlist=1&playlist=1.

A # in the id (myitem#frag&playlist=1) lands the params inside the fragment, so they never reach archive.org.

Running $extra_query through wp_parse_str() and merging it into $query_args before a single add_query_arg() call would resolve all of these in one path — add_query_arg() is fragment-aware, so it'd even place the query before any # — and the ?/& ternary could go away. Current behavior is correct for the common cases, so no need to block on this.

Decided to fold this in now since the diff is fresh — 699ae7c parses the trailing query string via wp_parse_str() and merges it into a single add_query_arg() call. That dedupes duplicate keys (shortcode atts win), and add_query_arg()'s fragment-awareness puts the query before any #frag in the id. Tests added for both cases.

- Re-check that the id is non-empty after splitting off query parameters, so identifiers like "?playlist=1" or "&playlist=1" return the missing-ID error comment instead of producing an item-less embed URL. - Parse query parameters baked into the id via wp_parse_str() and merge them into a single add_query_arg() call, which dedupes duplicate keys and stays fragment-aware so "#frag" placed in the id no longer swallows the query string.

zdenys · 2026-05-30T06:22:01Z

Thank you for your thorough review and comments, @kraftbj 🙌

PHPCS (MediaWiki.PHPUnit.AssertEquals.Int) prefers assertSame over assertEquals for non-numeric or strict equality checks.

zdenys added Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. labels May 27, 2026

zdenys self-assigned this May 27, 2026

github-actions Bot added [Feature] Shortcodes / Embeds [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Tests] Includes Tests labels May 27, 2026

zdenys requested review from jeherve and kraftbj May 27, 2026 11:43

Shortcodes: fix poster URL assertion in archiveorg test

df84149

add_query_arg() does not urlencode values, so the poster URL is embedded verbatim in the query string.

kraftbj requested a review from Copilot May 29, 2026 15:31

Copilot started reviewing on behalf of kraftbj May 29, 2026 15:31 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread projects/plugins/jetpack/tests/php/modules/shortcodes/Jetpack_Shortcodes_ArchiveOrg_Test.php

kraftbj reviewed May 29, 2026

View reviewed changes

kraftbj added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels May 29, 2026

Shortcodes: use assertSame for the dedup count in archiveorg test

7c7b30a

PHPCS (MediaWiki.PHPUnit.AssertEquals.Int) prefers assertSame over assertEquals for non-numeric or strict equality checks.

zdenys added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels May 30, 2026

zdenys requested a review from kraftbj May 30, 2026 07:22

jeherve removed the [Status] Needs Review This PR is ready for review. label Jun 2, 2026

jeherve added the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 2, 2026

jeherve approved these changes Jun 2, 2026

View reviewed changes

kraftbj merged commit eb2fdf3 into trunk Jun 2, 2026
105 of 106 checks passed

kraftbj deleted the fix/archiveorg-shortcode-query-parameters branch June 2, 2026 20:29

github-actions Bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 2, 2026

Conversation

zdenys commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Related product discussion/links

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

Testing instructions

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp-launch-control Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

zdenys commented May 27, 2026

Uh oh!

zdenys commented May 27, 2026

Uh oh!

zdenys commented May 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

kraftbj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraftbj May 29, 2026

Choose a reason for hiding this comment

Uh oh!

zdenys May 30, 2026

Choose a reason for hiding this comment

Uh oh!

kraftbj May 29, 2026

Choose a reason for hiding this comment

Uh oh!

zdenys May 30, 2026

Choose a reason for hiding this comment

Uh oh!

zdenys commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zdenys commented May 27, 2026 •

edited

Loading

github-actions Bot commented May 27, 2026 •

edited

Loading

github-actions Bot commented May 27, 2026 •

edited

Loading

jp-launch-control Bot commented May 27, 2026 •

edited

Loading

kraftbj left a comment •

edited

Loading