#18180 closed task (blessed) (fixed)
Missing i18n in setup-config.php
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (37)
#2
@
13 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.
#3
@
13 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.
#6
@
13 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.
#8
@
13 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.
#9
@
13 years ago
Some suggestions:
- Since we already have
version.phpincluded, couldn't we just use$wp_local_packageif it's set? $text_directionis set inWP_LANG_DIR . "/$locale.php", so I guess we need to include that file if it exists.install.cssis changed toinstall.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 inDB_NAME,DB_USERandDB_PASSWORD. We also need to applystripslashes()before trying to connect to a database. - Due to [18578], we need to set
<body class="rtl">in bothsetup-config.phpandinstall.phpifis_rtl(). We don't have that function insetup-config.php, so we can check$text_directionitself.
Uploaded 18180.3.diff for further review.
#10
@
13 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)
#11
@
13 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?
#13
@
13 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.
#14
follow-up:
↓ 15
@
13 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
#15
in reply to:
↑ 14
@
13 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.
#16
@
13 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()
#19
@
13 years ago
- Milestone changed from Future Release to 3.4
- Type changed from enhancement to task (blessed)
#20
@
13 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.
#21
@
13 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 ) )
#26
@
13 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.
#28
@
13 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 beforeprefixininstall.dev.css - Set
dir="rtl".language_attributes()doesn't exist at this point. - Set
class="rtl"(like ininstall.php).
wp-admin/setup-config.phpshould be translated directly, as stated in Codex:http://codex.wordpress.org/Files_For_Direct_Translation