WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18180 closed task (blessed) (fixed)

Missing i18n in setup-config.php

Reported by: CoenJacobs Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.2.1
Component: I18N Keywords: has-patch
Focuses: Cc:

Description

Three wp_die( strings need some i18n magic.

Attachments (7)

18180.diff (1.9 KB) - added by CoenJacobs 3 years ago.
18180.2.diff (17.2 KB) - added by dd32 3 years ago.
18180.3.diff (17.9 KB) - added by SergeyBiryukov 3 years ago.
18180.4.diff (17.9 KB) - added by dd32 3 years ago.
18180.5.diff (3.2 KB) - added by nacin 2 years ago.
18180.6.diff (32.8 KB) - added by nacin 2 years ago.
Introduces wp_load_translations_early() and leverages it.
18180.7.diff (1.3 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (37)

CoenJacobs3 years ago

comment:1 SergeyBiryukov3 years ago

wp-admin/setup-config.php should be translated directly, as stated in Codex:
http://codex.wordpress.org/Files_For_Direct_Translation

comment:2 CoenJacobs3 years ago

  • Keywords has-patch needs-testing removed
  • Resolution set to invalid
  • Status changed from new to closed

Correct, think I missed that Codex page.

comment:3 nacin3 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Resolution invalid deleted
  • Status changed from closed to reopened

I'd like to change this, so re-opening.

comment:4 nacin3 years ago

  • Owner set to nacin
  • Status changed from reopened to reviewing

comment:5 SergeyBiryukov3 years ago

  • Resolution set to duplicate
  • Status changed from reviewing to closed

comment:6 nacin3 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 3.3
  • Resolution duplicate deleted
  • Status changed from closed to reopened

Actually, let's re-open this. #18200 is going to get pretty hairy and will need tickets spun off.

comment:7 jottlieb3 years ago

  • Cc jottlieb@… added

comment:8 dd323 years ago

Whilst working on #13839, #14516 and altering the usage of wp_die due to #17975, I got carried away and ended up rewriting setup-config.php in a large way, So much so, that native translations ( __() & _e() ) can be utilised instead of the replacement system currently in use.

Aside from the method used for detection of the language to load, it's pretty solid..

Attaching a diff here for review.

dd323 years ago

SergeyBiryukov3 years ago

comment:9 SergeyBiryukov3 years ago

Some suggestions:

  • Since we already have version.php included, couldn't we just use $wp_local_package if it's set?
  • $text_direction is set in WP_LANG_DIR . "/$locale.php", so I guess we need to include that file if it exists.
  • install.css is changed to install.dev.css. Is that intentional?
  • Do we actually need to allow apostrophes in DB_HOST? We didn't touch it in #13839, so I guess we should allow them only in DB_NAME, DB_USER and DB_PASSWORD. We also need to apply stripslashes() before trying to connect to a database.
  • Due to [18578], we need to set <body class="rtl"> in both setup-config.php and install.php if is_rtl(). We don't have that function in setup-config.php, so we can check $text_direction itself.

Uploaded 18180.3.diff for further review.

comment:10 dd323 years ago

Since we already have version.php included, couldn't we just use $wp_local_package if it's set?

To be honest, I didn't even look at what that variable contains since I was developing this on a english-only install.

$text_direction is set in WP_LANG_DIR . "/$locale.php", so I guess we need to include that file if it exists.

Ah, that's where it's set, I thought it might've been set through one of the helper functions - It's not really something that -needs- to be set through PHP to start with. The Internationalisation included was really bare-basics in that patch, it's not supposed to be a final clean patch, rather a draft of what we could do.

install.css is changed to install.dev.css. Is that intentional?

That'd change when commited, the patch includes changes to the dev stylesheet, so for testing, simpler to include that

Do we actually need to allow apostrophes in DB_HOST?

Lets leave that for #13839 - but it was purely a consistency thing.

Due to [18578], we need to set <body class="rtl"> in both setup-config.php and install.php if is_rtl(). We don't have that function in setup-config.php, so we can check $text_direction itself.

We can include locale.php (Where that's set) most likely, however we don't have language_attributes() either, so the text direction needs to be set as well (not just the css class)

dd323 years ago

comment:11 dd323 years ago

Duplicated some of your efforts SergeyBiryukov, but took all that on board, Patch still contains parts of #13839 and some dev stuff (as it's a work in progress).

I've removed some translations of things that shouldn't be translated (wordpress, localhost(That could be translated, but I doubt it'd resolve on sites..), and wp_ (the table prefix).

The latest patch doesn't require any of setup-config.php to be translated manually at all, which is always a win.

I investigated including is_rtl(), but that ultimately leads to requiring additional files which require WordPress to be installed, the manual method is easier, I've also sync'd it all to work the same way as the WordPress admin/wp-settings.php does when including and/or defining.

The only other thing which has come to mind, Is do we want to have the installer (both setup config, and install.php) in the default language files? Or do we want to separate it like the MultiSite strings? But that can be changed later if need be.

@nacin: Got any thoughts on this? Sound like the way forward to you? Am I missing anything here which affects transations?

Last edited 3 years ago by dd32 (previous) (diff)

comment:12 dd323 years ago

In [18622]:

Mark the install page as rtl for styling purposes. Props SergeyBiryukov. See #18314 and #18180

comment:13 SergeyBiryukov3 years ago

While testing RTL on he_IL package, I've noticed that they use some additional styles in setup-config.php:

<link rel="stylesheet" href="../wp-content/languages/he_IL.css" type="text/css" />
<style media="screen" type="text/css">
<!--
input {direction: ltr;}
-->
</style>

http://svn.automattic.com/wordpress-i18n/he_IL/tags/3.2.1/dist/wp-admin/setup-config.php

Perhaps we can use wp_enqueue_style()/wp_print_styles() or provide some other action to be leveraged in WP_LANG_DIR . "/$locale.php" for such cases.

Version 0, edited 3 years ago by SergeyBiryukov (next)

comment:14 follow-up: dd323 years ago

Odd, RTL langage and admin, yet ltr inputs in the installer? seems quite strange to me.

Part of the changes required adding plugin.php, so we've now got actions and filters available, so we can put a action in the header for output of things like that if need be.

he_IL should definitely be using wp_enqueue_style() for all non-installer locations however (Including those functions in the setup-config section is going to be too reliant upon url-related functions which are unavailable)

Another one is we need to check the 'put your unique string' here consistency, Some languages translate it, some don't, will need to ensure that it's always set properly (we could just do a replacement for the english, AND the translated version even to be safe)

sv_SE also translates the default database name as well (I removed translation for that field in the last revision): http://svn.automattic.com/wordpress-i18n/sv_SE/trunk/dist/wp-config-sample.php

comment:15 in reply to: ↑ 14 RanYanivHartstein3 years ago

Odd, RTL langage and admin, yet ltr inputs in the installer? seems quite strange to me.

The inputs in setup-config.php all ask for server information, so it will always be LTR. Or anyway I can't think of a situation where it would need to be RTL. Everything else is RTL though.

he_IL should definitely be using wp_enqueue_style() for all non-installer locations however (Including those functions in the setup-config section is going to be too reliant upon url-related functions which are unavailable)

Not following - what should be changed?

I haven't updated the setup-config.php file yet, so anything that's now in he-IL might not relevant to what this ticket is discussing.

comment:16 dd323 years ago

The inputs in setup-config.php all ask for server information, so it will always be LTR

I had just assumed that a RTL language would be written RTL regardless of the data type, take for example, the Permalinks structure or Post to Email (first fields that comes to mind) they're rtl'd too?

Not following - what should be changed?

Not in regards to setup-config.php there, specifically referring to using wp_enqueue_script() in wph_admin()

comment:17 nacin3 years ago

In [18897]:

Revert [18817]. Adding the new HTML to setup-config would make translation files stale, and I'd rather i18n that file first (see #18180). fixes #18865, see #16413.

comment:18 ryan3 years ago

  • Milestone changed from 3.3 to Future Release

Punting from 3.3 per bug scrub.

comment:19 nacin2 years ago

  • Milestone changed from Future Release to 3.4
  • Type changed from enhancement to task (blessed)

nacin2 years ago

comment:20 nacin2 years ago

18180.5.diff makes setup-config.php agnostic to the default values for the various constants, such as "database_name_here", "password_here", and "put your unique phrase here". That means that once the strings are i18n'd, translators will no longer need to touch setup-config.php to make it match any wp-config-sample.php changes.

comment:21 dd322 years ago

18180.5.diff makes setup-config.php agnostic to the default values for the various constants, such as "database_name_here", "password_here", and "put your unique phrase here".

Looks good/sane to me, I was thinking we might want to make the expression a bit more accepting, but i'm not entirely sure it'll be needed at all:

if ( ! preg_match( '/^define\(\s*[\'"]([A-Z_]+)[\'"]\s*,([ ]+)/', $line, $match ) ) 

comment:22 nacin2 years ago

In [19701]:

Use regex to fill in config-sample. Prevents translators from needing to manually translate 'database_name_here' (and friends). see #18180.

nacin2 years ago

Introduces wp_load_translations_early() and leverages it.

comment:23 nacin2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [19760]:

Introduce wp_load_translations_early(), which can be used before the locale is properly loaded in order to translate early error strings. Internationalize setup-config.php -- translators no longer have a reason to modify this file. fixes #18180.

comment:24 nacin2 years ago

In [19761]:

Docs for WP_Locale::is_locale_rtl( $locale ). see #18180, see #19600.

comment:25 nacin2 years ago

In [19787]:

Load admin-$locale.mo in setup-config.php. see #19852, see #18180.

comment:26 nacin2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Looks like there are a few RTL and he_IL things we need to take care of, based on earlier comments.

comment:27 nacin2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [19789]:

LTR for fields on setup-config.php. see #19603, fixes #18180.

SergeyBiryukov2 years ago

comment:28 SergeyBiryukov2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This needs some more work for RTL, see 18180.7.diff.

  • Add missing # sign before prefix in install.dev.css
  • Set dir="rtl". language_attributes() doesn't exist at this point.
  • Set class="rtl" (like in install.php).

comment:29 nacin2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [19860]:

Add some RTL misses for setup-config. props SergeyBiryukov, fixes #18180.

comment:30 nacin2 years ago

In [19862]:

Use is_rtl() for html direction when possible. In sites.php, language_attributes() will always exist (MU vestige). When the DB is dead, language_attributes() is worse than a simple is_rtl() check. is_rtl() exists here due to wp_load_translations_early(). see #18180.

Note: See TracTickets for help on using tickets.