Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#50732 closed defect (bug) (fixed)

Block Directory: Support installing blocks in languages other than English

Reported by: ryelle's profile ryelle Owned by: whyisjake's profile whyisjake
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.5
Component: Editor Keywords: has-patch
Focuses: Cc:

Description

Currently, the block directory does not respect the site's locale when adding new blocks.

  1. Use a language other than English (Spanish works for this example)
  2. Search for a block with a Spanish translation (ex, Simple iframe)
  3. Everything so far is using Spanish translations ✅
  4. Click Add Block
  5. The block is added, but the placeholder & controls are all in English

Reloading the page shows the content correctly translated in Spanish, the issue is that the Block Directory is not loading in the translation data through setLocaleData.

Change History (21)

#1 @whyisjake
5 years ago

  • Milestone changed from Awaiting Review to 5.5

#2 @ryelle
5 years ago

One solution is to call setLocaleData in the JS that loads the block assets. We would also need the json translation content to pass in here for each JS file (probably generated on the WP.org side), and passed through the REST API. Then we could add something like this to the loadScript for each asset — assuming url is the URL to the respective json translation file, and that we have the text domain (also needed from the API).

const translations = await apiFetch( { url } );
const localeData =
	translations.locale_data[ domain ] ||
	translations.locale_data.messages;
localeData[ '' ].domain = domain;
setLocaleData( localeData, domain );

I can work on a PR tomorrow morning (US), if this sounds like a path forward.

#3 @swissspidy
5 years ago

  • Keywords needs-patch added

Ugh, bummer that this was missed :-(

Not sure how the block installation currently works, but here are my two cents:

First of all, the translations should get installed together with the block (if not already the case). The text domain should be known at that point (part of block.json, equals the plugin slug, etc.)

Second, we should not fetch the JSON file directly as it prevents legitimate use cases where the JSON translations are filtered by developers - see load_script_translations and the pre_load_script_translations and load_script_translation_file filters.

So we indeed need to go through the REST API, ideally via a GET /wp/v2/script-translations?handle=foo&locale=es_ES route that calls WP_Scripts::get_translations (like WP_Scripts::print_translations but without the printing) which calls load_script_textdomain with the right arguments.

#4 @tellyworth
5 years ago

Another possibility worth exploring is, the remote w.org API could return an additional (or replacement) JS asset file, a kind of shim. The shim would load (or contain) the translations before injecting the main block JS.

It's hacky and definitely not Plan A, but we're experimenting with the idea in parallel with other more complete solutions.

This ticket was mentioned in Slack in #polyglots by evarlese. View the logs.


5 years ago

#6 @dd32
5 years ago

I think it's worth noting that wp.i18n translations aren't the only problem that ideally needs to be solved here. In short:

  • Plugins that make use of wp_localize_script() or wp_add_inline_script() don't work
  • Plugins that depend upon other JS/CSS assets may not work (ie. Core-provided JS asset that isn't auto-enqueued on the editor screen, or, a non-editor-related JS script registered by the plugin)
  • By simply injecting CDN'd JS/CSS blindly that WordPress.org has provided for the block, it introduces a reliance upon the WordPress.org CDN even though the site has the files locally, a Content-Security-Policy header may even prohibit the CSS/JS loading, and in other cases, the WordPress.org CDN may be blocked by "network filters"
  • By having WordPress.org provide the assets, it limits the ability for other plugins to hook in / replace the API call to provide their own Block Directory implementations, while not impossible, it means they'd have to do a bunch of the work we're doing on WordPress.org in order to detect the Block assets ahead of time.

With respect to the first two points, the blocks will work fine if added via Plugins -> Add New, or the page is reloaded after using the Block Installer.

Ideally, WordPress.org would not need to provide the list of JS/CSS assets to load, as it should be defined in the block.json and Gutenberg would simply load the assets based on a script/style dependancy check, ie. using #48654.

Given we're already performing an expensive operation (Installing a plugin) I don't think we need to be too concerned with speed, but rather for editor safety and aim for a seamless always-works solution.

I'd like to propose a different option, instead of having WordPress.org provide the assets, instead of requiring the authors properly define their asset requirements in the block.json, and instead of reloading the editor after each Block install, instead we re-load the editor interface through a XHR request (HTML only) and inject any differences in loaded styles/scripts into the current editor screen. In my testing, this works well, although has slightly more latency.

https://github.com/WordPress/gutenberg/pull/24117 is an attempt at doing exactly that:

  • Install Block
  • Request current page via XHR
  • Insert any stylesheets or JS (either external, or inline JS - ie. Translations) into the current document
  • Continue inserting the block.

In my testing it adds a slight delay (~2s on my development machine), as it's another admin page view, but could be optimised in the future by treating this as a fallback only when the assets aren't defined in the block.json.

#7 @whyisjake
5 years ago

Say the page is reloaded, does it come back in the correct language, regardless of how it was installed?

#8 @ryelle
5 years ago

Say the page is reloaded, does it come back in the correct language, regardless of how it was installed?

Yeah, a full page reload will come back with everything in the correct language (unless the plugin isn't loading translations correctly, which is also an issue with some block-directory blocks).

#9 @TimothyBlynJacobs
5 years ago

In my testing it adds a slight delay (~2s on my development machine), as it's another admin page view, but could be optimised in the future by treating this as a fallback only when the assets aren't defined in the block.json.

We'd also be able to improve this when we land the scripts & styles endpoint. Which I think we want to do in 5.6

This ticket was mentioned in PR #427 on WordPress/wordpress-develop by dd32.


5 years ago
#10

  • Keywords has-patch added; needs-patch removed

REST API: Install plugin translations after the plugin install. This only installs for the plugin in question, not all plugins.

Support for retrieving the langauge pack alongside the install API request was added in https://meta.trac.wordpress.org/changeset/10091 to avoid having to make a plugin update check during the REST API check.

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

#11 @dd32
5 years ago

PR 427 installs the translation language packs for a plugin when a plugin is installed via the REST API.

I specifically added language pack data to the plugin information endpoint to allow installing a singular language pack, rather than the default behaviour of the Language Pack Upgrader which installs every language update for all items (core/plugins/themes) and can only get the information once the update_plugins API call is made, all of which would've slowed the API endpoint down too much IMHO.

That works in combination with https://github.com/WordPress/gutenberg/pull/24117 well in my testing, however, I'm struggling to find blocks which have translations and properly call wp_set_script_translations(), so testing that translations load has been difficult.

#12 @tellyworth
5 years ago

I think this should go in for RC1. It's been tested by me, @dd32, @ryelle, and @dufresnesteven.

PR 427 is needed for any solution. @TimothyBlynJacobs any thoughts on that?

Gutenberg PR 24117 is the preferred solution for the front end. That's tagged to backport to core along with other editor fixes going in to RC1.

API changes on the meta side have already been deployed.

This ticket was mentioned in Slack in #core-editor by whyisjake. View the logs.


5 years ago

#14 follow-up: @TimothyBlynJacobs
5 years ago

Makes sense to me! Should we unhook Language_Pack_Upgrader::async_upgrade here too?

#15 @whyisjake
5 years ago

  • Owner set to whyisjake
  • Resolution set to fixed
  • Status changed from new to closed

In 48641:

REST API: Install plugin translations after the plugin install. This only installs for the plugin in question, not all plugins.

Support for retrieving the langauge pack alongside the install API request was added in https://meta.trac.wordpress.org/changeset/10091 to avoid having to make a plugin update check during the REST API check.

Fixes #50732.
Props dd32, ocean90, ryelle, swissspidy, tellyworth, whyisjake, TimothyBlynJacobs.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#17 in reply to: ↑ 14 @dd32
5 years ago

Replying to TimothyBlynJacobs:

Makes sense to me! Should we unhook Language_Pack_Upgrader::async_upgrade here too?

Just noting that it shouldn't be hooked on this API, it's only hooked when in the admin:

wp-admin/includes/admin-filters.php:
add_action( 'upgrader_process_complete', array( 'Language_Pack_Upgrader', 'async_upgrade' ), 20 );

Cron tasks for automatic updates don't have it hooked either, as it performs the process "manually".

dd32 commented on PR #427:


5 years ago
#18

Merged in beba4bb73a10b97809105b3184da7006fbe25f55

#19 @TimothyBlynJacobs
5 years ago

Just noting that it shouldn't be hooked on this API, it's only hooked when in the admin:

Ah, gotcha. Thanks for clarifying!

#20 @TimothyBlynJacobs
5 years ago

In 48656:

REST API: Remove assets field from block directory controller.

Gutenberg no longer uses the assets field to fetch the assets for the installed block so this field can be dropped from the endpoint. This allows us to reintroduce it at a later point without needing to worry about backward compatibility.

See #50732.

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


5 years ago

Note: See TracTickets for help on using tickets.