site-kit-wp: Plugin translations not rendering fully translated strings
Bug Description
As mentioned in a WordPress support topic some translations may not be rendering within the plugin, despite these translation strings existing and marked as complete.
An example of a string which should be fully translated in various languages is below (Site Kit > Settings > Connect More Services): “Connect More Services to Gain More Insights”
Example below:
-
Spanish (Spain) - marked as 100% translated. “Connect More Services to Gain More Insights” string translated but not rendering
-
Bahasa Indonesian - marked as 87% translated.
“Connect More Services to Gain More Insights” string translated but not rendering
Note also there are inconsistent results when checking from a clean WordPress install, with even less translations appearing:
Steps to reproduce
- Switch language to Spanish in “General” settings
- Visit Site Kit dashboard > Settings > Connect More Services (or any other pages with considerable content / strings)
- Translations not always appearing
Additional Context
- Link to current translations & status
- Multiple languages tested with the same result, including languages with 100% completion
- When checking from other websites I encounter inconsistent translations. (One site has some Bahasa Indonesia translations which another doesn’t)
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Plugin translations for JavaScript files should be loaded and used as expected for WordPress versions >= 5.0.
- The existing broken approach where PHP translations are passed to JavaScript should be removed.
- It should be kept in mind that the plugin supports older WordPress versions up until 4.7, so newer core functions that are needed for translating JS file strings should only be called if they exist.
A note on testing
- The translation files that wordpress.org generates and that WordPress loads are based on md5 hashes based on the JS file names.
- Since our JS file names change between almost every build, testing this won’t work outside of an actual release without the following workaround:
- We put together a mini plugin (outside of Site Kit) that uses the WordPress core filter
load_script_textdomain_relative_path
. Download mini plugin here: https://gist.github.com/adamsilverstein/3eee29ea1370d0e119bb929a331e9f50 - This filter allows to modify for which file path to look for a corresponding translation file.
- We know the current file paths via the generated
Manifest
class - so we need to use the filter to replace all of those with the corresponding file paths from the latest release (for which we have generated translation files downloaded) - i.e. we need a map ofcurrentBuildFilePath => latestReleaseFilePath
. - We can either use some approach where we download the
Manifest
class from the latest release (e.g. https://plugins.trac.wordpress.org/browser/google-site-kit/trunk/includes/Core/Assets/Manifest.php) and parse out the contents (would work as a long-term workaround), or we can do something simpler for once-off testing where we just manually copy the file paths from the latest release into that mini plugin to make it work.
- We put together a mini plugin (outside of Site Kit) that uses the WordPress core filter
Implementation Brief
- Create our own entrypoint that exposes a global
googlesitekit.i18n
which is our own internalized version of@wordpress/i18n
(since we want to avoid version conflicts and therefore bundle our own). - Register that asset in PHP as
googlesitekit-i18n
and make it a key dependency of pretty much everything (add it to$dependencies
array, but also to other key dependencies that use translation functions). - Add an external for
@wordpress/i18n
to Webpack that will make it usegooglesitekit.i18n
in production. - Add a private method
set_locale_data
to theScript
class, which should be our own variant of WordPress core’sWP_Scripts::print_translations
method.- It should use
wp_add_inline_script
with a script that callsgooglesitekit.i18n.setLocaleData
(instead ofwp.i18n.setLocaleData
). The$position
argument should be set tobefore
so that the translation data inline script is output before the actual script. - It should rely on core’s
load_script_textdomain
- for BC though, add this as a protected static method on ourBC_Functions
utility class and call it there. If the function doesn’t exist, it should be implemented as a no-op that returnsfalse
(the same as if core didn’t find any translations). - If the call to
BC_Functions::load_script_textdomain
returns a false-y value, simply bail and don’t add the inline script.
- It should use
- In the constructor of the
Script
class, “reassign”$this->args['before_print']
to a callback (using a closure) that first calls the newset_locale_data
method and then calls the originally passedbefore_print
callback (if any). See theScript_Data
constructor for a somewhat related example. - Remove usage of
_googlesitekitLegacyData.locale
. - In our JS codebase, remove the
loadTranslations
utility function and all calls to it.setLocaleData
is now called via the inline scripts in PHP.
Test Coverage
- N/A
Visual Regression Changes
- N/A
QA Brief
- Go to the
Settings
>General
page and selectEspanol
as your site language. - Go to the
Dashboard
>Updates
page (/wp-admin/update-core.php
) and click on theUpdate Translations
button. After doing it, WordPress will download spanish translations for the plugin. - Install and activate the following plugin: google-site-kit-language-loader.zip
- Go to the plugin pages and make sure the interface is translated into Espanol.
Changelog entry
- Fix JavaScript translations that were not appearing to work correctly, given the site uses WordPress >= 5.0, which is required for support of JavaScript translations.
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 36 (8 by maintainers)
Commits related to this issue
- Fix locale lookup (see #2171). — committed to google/site-kit-wp by felixarntz 4 years ago
- Fix broken Storybook stories by updating Storybook legacy data after server-side change (see #2171). — committed to google/site-kit-wp by felixarntz 4 years ago
To clarify: The reason that right now some strings are still translated in JS even though JS translations don’t work is that we currently pass PHP translation data to JS (via
_googlesitekitLegacyData.locale
global). This was implemented a long time ago, probably without awareness that that data only includes the PHP translation data, but not the JS translation data. For example, we have a__( 'Settings', 'google-site-kit' )
in both PHP and JS, so that string will be “randomly” translated in JS.There’s another complexity here which is that Site Kit has a minimum requirement of WordPress 4.7 where all those PHP functions didn’t exist at all. We can certainly rewrite our own version of
WP_Scripts::print_translations
, but I think rewriting/backporting functions likeload_script_textdomain
andload_script_translations
would get a bit too crazy.After further discussion with @aaemnnosttv and @tofumatt, I think we have two viable options here:
If we decide it’s okay to not give JS translation support to pre-5.0 versions
googlesitekit.i18n
which is our own internalized version of@wordpress/i18n
(since we want to avoid version conflicts and therefore bundle our own).googlesitekit-i18n
and make it a key dependency of pretty much everything.@wordpress/i18n
to Webpack that will make it usegooglesitekit.i18n
in production.WP_Scripts::print_translations
which useswp_add_inline_script
with a script that callsgooglesitekit.i18n.setLocaleData
. It will rely on core’sload_script_textdomain
- if that function doesn’t exist, it simply won’t provide JS locale data._googlesitekitLegacyData.locale
.setLocaleData
in our JS code because it’s now handled via the above approach usingwp_add_inline_script
.If we decide we need to fully support pre-5.0 versions despite technical drawbacks
@felixarntz I created a mini plugin that loads the correct language files - https://gist.github.com/adamsilverstein/3eee29ea1370d0e119bb929a331e9f50
I included the manifest mapping manually for now to keep things simple, this would need to get updated with each release. Testing in my local, once I added the
wp_set_script_translations( $this->handle, 'google-site-kit' );
line after enqueueing our scripts, WordPress properly found the translation files installed from the last release.@felixarntz IB looks solid, just a few details to clarify:
Script::set_locale_data
be called from?$position
forwp_add_inline_script
providing translations isbefore
rather than the defaultafter
@mushlih-almubarak We are still working on a fix for this issue, thanks for your patience!
@mushlih-almubarak Rest assured we are currently investigating this. Keep watch of this issue whereby we hope to have further updates this week.