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

Ensure correct translation loading when loading assets via CDN #14797

Merged
merged 3 commits into from
Mar 5, 2020

Conversation

akirk
Copy link
Member

@akirk akirk commented Feb 25, 2020

When activating the CDN, the Jetpack blocks don't load their translations properly. This is because Core language loading assumes local files. This fixes the paths under the hood before the JSONs are loaded.

Testing instructions:

  1. Change your UI language to something other than English (with sufficient translation percentage, e.g. German, Spanish, or French).
  2. Activate Jetpack CDN in the Jetpack Settings.
  3. Create a new post with the Gutenberg editor.
  4. Click "Add New Block" and scroll down to the Jetpack blocks. Block names and descriptions should be translated.

Proposed changelog entry for your changes:

  • Fixed Translation loading of Gutenberg blocks with activated CDN.

@akirk akirk added the [Focus] i18n Internationalization / i18n, adaptation to different languages label Feb 25, 2020
@akirk akirk self-assigned this Feb 25, 2020
public static function fix_local_script_translation_path( $file, $handle, $domain ) {
switch ( $handle ) {
case 'jetpack-blocks-editor':
return str_replace( 'wp-content/languages/jetpack', 'wp-content/languages/plugins/jetpack', $file );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hairy thing here is that we need to rewrite only the plugin translations. It looks like core assumes that these CDN-ed plugins have their translations in the core folder and not in the plugins folder.

@jetpackbot
Copy link

jetpackbot commented Feb 25, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: March 3, 2020.
Scheduled code freeze: February 25, 2020

Generated by 🚫 dangerJS against 434cf50

@akirk akirk added the [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins label Feb 25, 2020
@akirk akirk added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label Feb 26, 2020
@akirk akirk marked this pull request as ready for review February 26, 2020 09:18
@akirk akirk requested a review from a team as a code owner February 26, 2020 09:18
@kraftbj kraftbj added [Feature] Asset CDN and removed [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Mar 5, 2020
Copy link
Contributor

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked well for me. If testing, be sure to have your translations packages fully updated and you're dropping the PHP on a stable version of the plugin so the CDN would fire for the assets.

@kraftbj kraftbj added the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 5, 2020
@kraftbj kraftbj added this to the 8.4 milestone Mar 5, 2020
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well on my end as well. Merging.

@jeherve jeherve merged commit 7988d52 into master Mar 5, 2020
@jeherve jeherve deleted the fix/cdn-script-translations-loading branch March 5, 2020 17:45
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 5, 2020
jeherve added a commit that referenced this pull request Mar 20, 2020
jeherve added a commit that referenced this pull request Mar 31, 2020
* Initial changelog entry

* Changelog: add #14904

* Changelog: add #14910

* Changelog: add #14913

* Changelog: add #14916

* Changelog: add #14922

* Changelog: add #14924

* Changelog: add #14925

* Changelog: add #14928

* Changelog: add #14840

* Changelog: add #14841

* Changelog: add #14842

* Changelog: add #14826

* Changelog: add #14835

* Changelog: add #14859

* Changelog: add #14884

* Changelog: add #14888

* Changelog: add #14817

* Changelog: add #14814

* Changelog: add #14819

* Changelog;: add #14797

* Changelog: add #14798

* Changelog: add #14802

* Changelog: add #13676

* Changelog: add #13744

* Changelog: add #13777

* Changelog: add #14446

* Changelog: add #14739

* Changelog: add #14770

* Changelog: add #14784

* Changelog: add #14897

* Changelog: add #14898

* Changelog: add #14968

* Changelog: add #14985

* Changelog: add #15044

* Changelog: add #15052

* Update to remove Podcast since it remains in Beta

* Changelog: add #14803

* Changelog: add #15028

* Changelog: add #15065

* Changelog:add #14886

* Changelog: add #15118

* Changelog: add #14990

* Changelog: add #14528

* Changelog: add #15120

* Changelog: add #15126

* Changelog: add #15049

* Chanegelog: add #14852

* Changelog: add #15090

* Changelog: add #15138

* Changelog: add #15124

* Changelog:add #15055

* Changelog: add #15017

* Changelog: add #15109

* Changelog: add #15145

* Changelog:add #15096

* Changelog:add #15153

* Changelog: add #15133

* Changelog: add #14960

* Changelog: add #15127

* Changelog: add #15056

* Copy current changelog to changelog archive.

* Clarify changelog description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Asset CDN [Focus] i18n Internationalization / i18n, adaptation to different languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants