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

RTL/LTR button missing from Paragraph block #28221

Closed
sirreal opened this issue Jan 15, 2021 · 8 comments · Fixed by #28231 or #28279
Closed

RTL/LTR button missing from Paragraph block #28221

sirreal opened this issue Jan 15, 2021 · 8 comments · Fixed by #28231 or #28279
Assignees
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release
Milestone

Comments

@sirreal
Copy link
Member

sirreal commented Jan 15, 2021

Description

The LTR button has gone missing from paragraph blocks in sites with RTL locales.

This button was added in #10663
This seems to be a regression from #27838

The isRTL check returns false, causing the LTR button not to render:

The following can also be observed in the JavaScript console, which suggests the issue is related to the locale:

wp.i18n.isRTL()
// false
// isRTL checks this under the hood
wp.i18n._x( 'ltr', 'text direction' )
// "ltr"

Yet, the locales I've tested appear to have the correct "ltr" setting, for ar and he.

Step-by-step reproduction instructions

  1. Create a site with Gutenberg 9.7 installed
  2. Set the site language to an RTL language, e.g. ar or he.
  3. Ensure translations are updated from the site dashboard
  4. Open the post editor
  5. Add a paragraph block
  6. The LTR button is missing

no-ltr

There should be an LTR button like this (installed Gutenberg 9.6 on the same site):

ltr with 9 6

Expected behaviour

LTR button should be present

Actual behaviour

LTR button is missing

WordPress information

  • WordPress version: 5.6
  • Gutenberg version: 9.7.1 (latest)
  • Are all plugins except Gutenberg deactivated? Yes
  • Are you using a default theme (e.g. Twenty Twenty-One)? Yes
@sirreal sirreal added Internationalization (i18n) Issues or PRs related to internationalization efforts [Type] Regression Related to a regression in the latest release labels Jan 15, 2021
@youknowriad
Copy link
Contributor

wp.i18n._x( 'ltr', 'text direction' )

It seems this should be returning "rtl" for these languages but when looking in glotpress, I don't see the string in the Gutenberg plugin https://translate.wordpress.org/projects/wp-plugins/gutenberg/stable/ar/default/?filters%5Bterm%5D=ltr&filters%5Bterm_scope%5D=scope_any&filters%5Bstatus%5D=current_or_waiting_or_fuzzy_or_untranslated&filters%5Buser_login%5D=&filter=Apply+Filters&sort%5Bby%5D=priority&sort%5Bhow%5D=desc

Could this be a bug with the string extraction. cc @swissspidy

@swissspidy
Copy link
Member

While files like block-library/dist/index.js will have strings like Object(we.__)("Image caption text") that can be detected, in i18n/dist/index.js it looks like this:

// ...
isRTL:function(){return"rtl"===i("ltr","text direction")}
// ...

There's no way string extraction can know that this is a translation call. Maybe you can tweak the webpack config to not mangle function names there or something?

@youknowriad
Copy link
Contributor

Interesting, I guess it's because the function is defined in the same module.

@sirreal
Copy link
Member Author

sirreal commented Jan 15, 2021

We came across an issue like this in Calypso and solved it by reserving translation functions from mangling. It would be a good idea for @wordpress/scripts to do the same.

// webpack config
optimization: {
	terserOptions: {
		mangle: { reserved: [ '__', '_n', '_nx', '_x' ] },
	},
}

@sirreal
Copy link
Member Author

sirreal commented Jan 15, 2021

I'll file a PR to reserve the names.

@sirreal
Copy link
Member Author

sirreal commented Jan 15, 2021

A patch release will be prepared for this, so the issue should be fixed in Gutenberg 9.7.3 or greater. It does depend on translations being extracted and then updated, however.

@sirreal
Copy link
Member Author

sirreal commented Jan 15, 2021

The fix in #28231 appears to have been insufficient. Although strings are now correctly extracted and translations have been updated and published with the appropriate translations for Gutenberg in he and ar locales, the wp-i18n script never appears to have translations attached in my testing and this issue remains.

@sirreal sirreal reopened this Jan 15, 2021
@sirreal sirreal added this to the Gutenberg 9.7 milestone Jan 15, 2021
@sirreal
Copy link
Member Author

sirreal commented Jan 15, 2021

These lines may be related:

/*
* `WP_Dependencies::set_translations` will fall over on itself if setting
* translations on the `wp-i18n` handle, since it internally adds `wp-i18n`
* as a dependency of itself, exhausting memory. The same applies for the
* polyfill script, which is a dependency _of_ `wp-i18n`.
*
* See: https://core.trac.wordpress.org/ticket/46089
*/
if ( 'wp-i18n' !== $handle && 'wp-polyfill' !== $handle ) {
$scripts->set_translations( $handle, 'default' );
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release
Projects
None yet
3 participants