Implement translation of customized 'translatable' views properties
| Project: | Views |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | i18n sprint, i18n views, views 3.x roadmap |
Various properties of views or displays - e.g., a view's header and footer strings - are designated as 'translatable'. Currently, the translatable attribute is used to decide which data should be wrapped in t() calls in exported views, so that they are properly processed for translation when used as default (code-based) views.
The translatable property was also intended as a a way to localize these admin-defined strings on a site when the view is not exported. Earl and I discussed ways we could complete this functionality--here are some notes from our conversation.
Currently the i18nstrings module is the most developed solution for localizing admin-defined strings via that core locale system. i18nstrings features a tt() function, analogous to core's t() but for user-defined strings.
The i18nviews module (like i18nstrings, part of the i18n package) has some code to facilitate translation of views data, but it is sketchy and doesn't use the translatable attribute.
Likely this would be best done or at least facilitated withing Views itself.
We would need a method to pull from a view all translatable properties and their values. This method would be similar to and probably based on the views_object unpack_options method, although it would need to be extended to also cover displays.
From here, we would need to respond on view save, delete, and display operations. We could either:
a. use hook invocations (introduce new ones for view save and delete) and rely on other modules to do the translation, or
b, probably better, implement this directly in Views
For b, we might consider simply using module_exists('i18nstrings') or, to have a more generic solution, allow other modules (e.g., i18nstrings) to register themselves as localization handlers.
#1
Here is an initial cut at a patch.
Approach:
* Introduce handlers for localization: 'none' (don't localize admin-defined strings), 'core' (use t()--not recommended, but left as the default in order not to break existing Views 2 D6 installs) and 'i18nstrings' (use the i18nstrings module, if enabled.). If i18nstrings is not enabled, don't show the i18nstrings option but do give a message about installing i18nstrings.
* Use drupal_alter() to allow other modules to alter or add localization handlers.
* Add a select to the Views tools configuration page to select which handler to use.
* Introduce a $view->editing property. This is needed because, when editing, we need to skip localization--otherwise, we'd get the translated data to edit, not the original.
* Change the unpack_options() method to localize admin-defined strings using the registered localization handler, if $view->editing == FALSE.
* Add an argument to unpack_options(), $localization_keys. We need a way to construct unique identifiers for each string to be localized. This will always begin with the view name, but also need, e.g., a display ID and a property name, e.g., for the header text in the default display of my_view: 'my_view:default:header. The $localization_keys in this case would be array('default'). They will be merged with the view name and the property name to become array('my_view', 'default', 'header').
If testing, make sure you *do not* have the i18nviews module installed, as that module will interfere with what we're doing here.
#2
Patch.
I'll follow up next week with testing suggestions.
#3
Ok, I agree with this direction. Here comes the big 'but'.
In the Views system, this would be a plugin. The plugins would be registered in hook_views_plugins just like all the other plugins, the plugin would have an object, and the base object would contain the basic API to make it work. views_plugin_localizer or something along those lines.
I'm guessing the selection of localizer should be site-wide and thus would be done on admin/build/views/tools and one and only one could be chosen.
I realize that's quite a lot more work but it is consistent with the rest of Views.
#4
Yes, that sounds right.
I'll work up a new patch. And I should be able to move the i18nstrings part to a patch on i18nstrings module, adding a views plugin there.
#5
Here's a reworked patch implementing the approach from #3.
All changes removed from views.module and moved into plugins. Yes, this is a definite improvement on the previous patch.
I'm not sure when best to initialize the localization plugin. I've done so in view::init() in view.inc. Maybe though it doesn't need to always be initialized? Only on display, saving, and deletion?
I'll work up an i18nstrings plugin in #360024: Write views localization plugin.
#6
This part of the patch hacks in a message about the Views translation module.
<?php+ // Add a help message pointing to the i18nstrings module if it is not present.
+ if (!module_exists('i18nviews')) {
+ $plugins['localization']['core']['help'] .= ' ' . t('If you need to translate Views labels into other languages, consider installing the <a href="!path">Internationalization</a> package\'s Views translation module.', array('!path' => url('http://drupal.org/project/i18n', array('absolute' => TRUE))));
+ }
?>
I couldn't come up with a better way of saying "t()'s not the best, here's an option".
#7
I've posted a patch at #360024: Write views localization plugin that can be tested with this patch.
#8
subscribing so I can review this later.
#9
Started testing this - but ran into an error fairly quickly (this is with this patch and the i18n patch both applied) when saving a view:
Fatal error: view::process_locale_strings() [function.view-process-locale-strings]: The script tried to execute a method or access a property of an incomplete object. Please ensure that the class definition "views_plugin_localization_core" of the object you are trying to operate on was loaded _before_ unserialize() gets called or provide a __autoload() function to load the class definition in /home/catch/www/6/sites/all/modules/views/includes/view.inc on line 1566This happens with both the 'none', 'core', and 'i18n_views' options.
I moved the call to $this->save_locale_strings(); above _save_rows(), and both the views and the strings seemed to save OK. But then on saving subsequent views I got the same error again, so that's clearly not the problem.
With the drupal_set_message() - I'd probably suggest putting that into advanced help files. Another option would be hook_requirements() - we could check for module_exists('locale') and maybe content translation then set a message if those are enabled by i18n_views isn't, but maybe advanced help would be enough.
#10
New patch with various changes and improvements, notably:
1. In several places, call $view->init_localization() to ensure there is a localization plugin in place.
2. Special handling for PHP code. The issue here is that sending PHP code for translation could (a) be confusing for translators, (b) create security holes, since translators would both have access to potentially sensitive data and insert changed PHP into translations.
Approach:
* If there is a filter format available for a translatable field, examine it to see if it includes the php filter.
* If so, extract PHP code and substitute with placeholders in the form !php0, !php1, etc. before passing for translation. The record in locales_source will have these placeholders rather than the original PHP. Translators will therefore not see the PHP.
* When displaying translated data, substitute the code back in. Example
If an English original view header is:
Hello, you are visitor number <?php echo visitor_count(); ?>.the string available for translation will be
Hello, you are visitor number !php0.This might be translated to Spanish as
Hola, ud. es el visitante numero !php0.
And the display in Spanish will be:
Hola, ud. es el visitante numero 524.
As well, we need to ensure that no new PHP is injected via translation. For this, I've used strip_tags(), because I didn't want to try to write a regular expression that would reliable strip all variations of PHP scripts. To address the potential need to retain some HTML, I've used a kinda hackish workaround: if there is an HTML filter as well as a PHP evaluator in the given filter format, allow any tags permitted by the HTML filter.
See the localization.html advanced help file for instructions on how this works.
This whole PHP handling approach needs review. It kinda works, but we might be able to do better.
3. As catch suggested, removed the module_exists('i18nviews') test, which is covered by what's in advanced help.
-----
The main remaining problem with the patch that I'm aware of is that it doesn't handle all 'translatable' properties. This is due to limitations in the option_definition() methods. These are needed in order to load information on which properties are translatable. However, not all translatable properties are loaded. For example, a display might include several fields that have translatable properties (e.g., labels). The $definition data returned by $this->option_definition() doesn't include these fields, and so their strings are not recognized as being translatable.
As well as extending the option definition methods to load additional data (e.g., fields), we would need to ensure that the these properties passed an appropriate array of keys to uniquely identify the strings. For example, a field value might be identified as: array('myviewname', 'mydisplayid', 'fields', 'myfieldname', 'myfieldproperty'). I've tried to write the methods so they will correctly extract and pass nested properties as an array of keys, but I haven't tested.
Thoughts on just leaving this problem of additional definition data for a follow-up patch?
#11
Updated #360024: Write views localization plugin, which can be used for testing.
#12
Yes, lets' do additional definition data as a followup patch.
#13
Initial review:
+ '#description' => t('Select a plugin for translation of Views data like header, footer, and empty text.'),Let's call this a method or something. Plugin isn't going to make a lot of sense to the user (we don't call styles style plugins to the user, only in code). Localization method seems pretty weak though. Ideas?
+ 'help' => t("Use Drupal core t() function. Not recommended, as it doesn't support updates to existing strings."),If this is not recommended, and none of the other options are available (i.e, i18n views isn't installed) then this is going to confuse a user. We should probably add a line recommending the i18n module.
+ return isset($this->type) && in_array($this->type, array('Normal', 'Overridden'));These strings are normally t()'d so they should be here too I think.
Otherwise, and I'm going to leave this to i18n experts to tell me, this is probably ready to go in as an initial "let's see what it can do".
#14
Subscribing
#15
subscribing
#16
This looks very promising.
+ case 'save':+ $this->localization_plugin->save($string, array_merge(array($this->name, $display_id), $keys));
+ break;
+ case 'save':
+ $this->localization_plugin->delete($string, array_merge(array($this->name, $display_id), $keys));
+ break;
The 2nd case should probably be 'delete'.
+ // Find PHP code.+ preg_match_all("/(<\?php)(.*?)(\?>)/", $value, $matches, PREG_SET_ORDER);
The closing PHP tag is optional.
+ return (array(+ // Whether this format accepts PHP.
+ isset($accepts_php) && $accepts_php,
+ // An array of PHP strings.
+ $search,
+ // An array of placeholders.
+ $replace,
+ // HTML tags allowed by this format, if any.
+ isset($allowed_html[$format]) ? $allowed_html[$format] : '',
+ ));
The surrounding braces around array() are superfluous here.
+<?php+// $Id: $
+/**
+ * @file
Minor: There should be a blank line between the CVS Id and @file.
+ function translate($string, $keys = array()) {...
+ function save($string, $keys = array()) {
I wonder whether we shouldn't pass a langcode to those functions? So we could add a 'preferred language' property to a view (display) in a separate step? (Sorry, I have no concrete use-case, but I think it would make sense.)
#17
Thanks for the reviews!
Here is a new patch incorporating all suggested changes, except sun's last suggestion. Good idea, but I'm not sure how we would implement it and, for now, the available handlers will assume a language ('en' for t(), the default language for tt()). Plus some bug fixes.
After discussing with merlinofchaos, I've moved the php handling to a helper module, proposed for i18n: #386454: Helper module to handle PHP in translations. This simplifies this patch and also makes sense because PHP handling in translations is an issue wherever there is a translatable string that has an associated filter format (like e.g. in custom blocks).
The help text for localization now refers to two potential helper modules, i18nviews and php_translation. The first is fine, since we'll apply the i18nviews patch as soon as this patch is applied. But the second might need changing, if it's decided that this helper module doesn't fit in i18n and should instead be a separate module.
I'll discuss with Jose Reyero and see what he thinks.
#18
subscribing and testing
#19
Sorry for being a dumb ass, but is there an easy way to test this patch without having to try and search and destroy the litte + signs everywhere?
I'm not a coder and I'm sure there is a cunning coder's tool that puts that kind of stuff straight in.
The reason is I would like to test this patch but it keeps failing.
However the reason it fails is because I can't put the patch in properly rather than there being a problem with the patch.
Now obviously code checkers like to see all the +/- for review but I would just like to plug it in and then try and break it. Any chance of just getting a zip file of the patched files?
Thanks,
Ben.
#20
Subscribing - being able to translate headers and footers will be very useful.
@egeos: Read http://drupal.org/patch/apply - there are programs which read and apply a patch file for you :)
#21
Thanks, I'll give it a go (^_^)b
#22
Oh for the King of the Fools!
I got the patch working :p
Eclipse was so surprisingly easy I didn't believe it actually worked.
However......... now (I think) I have the patch working, I have nooooooooo idea what it does (which is rather embarrassing)
My main intent was to be able to switch the Title of a view to display in the language it was being viewed in (same for header and footer).
I recently learnt a cunning trick to change the title by putting PHP in the header; but I thought this patch somehow allows the Views display fields to be translatable?
eg. I open up the header........ and like content I can bash 'translate' and write the header in a different language.
Same with title: I write the title in for the default language and then for example bash the little cog widget........choose another language and write in my title.
Is this what this patch is designed to do because I think I have no idea how I should use the patch.
Could someone tell me how to test it? ;)
#23
subscribe
#24
One weak place in the patch is the way I've invoked the PHP string handling. We need a pair of hook implementations. The first (a) strips out PHP, replacing with placeholders and (b) returns data including the PHP that was stripped out. The second takes the data returned by the first and swaps back the PHP for the placeholders.
I wanted to use drupal_alter() but it's a string that's being altered so there appeared to be no way to add more data parameters (the removed PHP, etc.). I looked at module_invoke_all(), but there we can't pass a string by reference. So I used a custom method (like node_invoke_nodeapi()) that both passes by reference and returns data. This approach requires modules using these hooks to repeat use some version of this custom method.
Now though I'm thinking we're probably better off passing the string an object, which would allow us to use drupal_alter(). Instead of the current patch's
<?php$this->invoke_translation_process($value, isset($options[$key . '_format']) ? $options[$key . '_format'] : NULL, 'pre');
?>
We would have:
<?php$string = new StdClass();
$string->string = $value;
drupal_alter($string, 'translation_process', isset($options[$key . '_format']) ? $options[$key . '_format'] : NULL, 'pre');
?>
Modules could then both alter the $string->string and make their own additions to the $string object.
#25
@egeos:
Good questions! It's not at all clear how to test this, so here are some quick pointers.
First, explanation. The patch makes it possible to select a method for translation of some views text (strings). Not all are yet supported. Header, footer, and empty text are supported.
Multiple methods are possible for translation. The patch itself only implements one, which is the "built-in strings" that are used for Drupal interface translation. The patch I put at #360024: Write views localization plugin provides a second method.
To test:
1. Apply the patch.
2. Enable views and locale modules.
3. Install and enable one language in addition to the default language English. Enable the language switching block.
4. Create a new view. Give it a header, e.g. 'My views header." Save the view.
5. Go to
Administer > Translate interface > Searchand search for the precise phrase you entered, making sure to use the same case. You should find that it has been added to the "Built-in interface" group. Enter and save a translation for the string in the non-English language you installed.6. Bring up the view and click on the language switcher link to switch to the non-English language. You should see that the header now displays the translated text.
7. Now download Internationalization and apply the patch at #360024: Write views localization plugin. Enable the views translation module and its dependencies.
8. At
Administer > Views > Tools, select "Views translation module" as the translation method.9. Repeat what you did in steps 4. Create a new view, give it a header, and save it. You should get a message that a new string has been created with the text of the views header.
10. Repeat what you did in step 5, searching for the view header text, except that now you should find it in the "views" group rather than the "built-in interface" one. Enter a translation, view the view, switch languages, and confirm that your translated header shows in the new language.
What's the difference between the two methods? The main practical difference is that the Views translation module approach supports updates and deletion. When you edit the view's header, the string will be updated in the interface translation system rather, whereas with the "built-in interface" approach a new record is created.
#26
Thanks for these tips - I was wondering the same.
#27
subscribing.
#28
subscribing
#29
subscribing
#30
Subscribing - should be testing this out in the next week or two
#31
I think this would be a great solution. I'll be committing the related i18n patch if this gets into views, #360024: Write views localization plugin
#32
The patch doesn't apply for the actual views 2.x
patch -p0 < views-localization-357529-17.patchpatching file views_ui.module
patching file help/localization.html
patching file help/views.help.ini
patching file includes/admin.inc
Hunk #1 FAILED at 2728.
Hunk #2 FAILED at 2977.
Hunk #3 FAILED at 2994.
3 out of 3 hunks FAILED -- saving rejects to file includes/admin.inc.rej
patching file includes/base.inc
patching file includes/plugins.inc
patching file includes/view.inc
patching file plugins/views_plugin_display.inc
patching file plugins/views_plugin_localization.inc
patching file plugins/views_plugin_localization_core.inc
patching file plugins/views_plugin_localization_none.inc
Maybe an small rebuild :)
#33
subscribe
#34
Yeah guys,
please make this happen!
#35
I've rerolled this patch for DRUPAL-6--3 with the following additions:
export()andexport_render()methods to the localization plugin for pushing translatables into exported Views.export_locale_strings()and theexport$op toprocess_locale_strings()$translatablesarray after each exported view. This allows translation extractors to find translatable strings without affecting the actual translatable stack present in Views.#36
Couple of small fixes... previous patch had some bad typos : )
#37
Hi:
I have tested the patch.
First, now apply correctly, thanks for the rollback.
Titles on tables are translated fine. However titles for labels in exposed filters are not being translated.
This patch should do this?
Changing status
#38
subscribe
#39
Tried patch #36, does not work when html tag present. Getting the error "The submitted string contains disallowed HTML".
Not sure this is locale or i18n problem, it seems it does not check the Input Format.
To replicate the problem, change the views header text, footer text, etc to Full HTML input format and put in some text with HTML tag.
#40
To translate label titles on exposed filters, the following change worked for me. I don't know whether it's the correct way to pick up label properties or not...
Line circa 101 of patched base.inc (DRUPAL-6--3) was
else if (!$this->view->editing && !empty($definition[$key]['translatable']) && !empty($value)) {changed to:
else if (!$this->view->editing && (!empty($definition[$key]['translatable']) || !empty($definition['contains'][$key]['translatable'])) && !empty($value)) {#41
Rollback of #36 with changes specified in #40. Now Titles for exposed filters #37 are translated correctly.
I also tested #39 and sorry, but I cant replicate this issue. I have created a header with Filtred HTML and Full HTML with this text:
testing <strong>html tags</strong>And its translation with "Translate Inteface"
probando <strong>etiquetas html</strong>And works fine.
Changing status.
#42
dagmar,
I believe what triggers the error message mentioned in #39 is html that is not allowed in the Filtered HTML input format, such as or tags.
#43
Clarification to #39, it is Full HTML input format under Views. I've tested with Filtered HTML (with allowed tags) also having same problem.
I'm getting the error not on the Views page, but on the Translate interface / Edit string page.
#44
To replicate the problem in #39, just put a html tag like div (in Full HTML or even after div is added to Filtered HTML)
[edited: warning no more for #41]
#45
I believe for this patch to truly work, we may need to get http://drupal.org/node/503452 in and ready because that will be required to fully translate exports. Am I right on this?
#46
adding a tag.
#47
Hi,
I'm very confused about this patch (#41). To what version should I apply it to? I have tried applying it to 6.x-2.x-dev as well as a few other versions but I keep getting hunk failures in certain files like admin.inc for example.
Please advise.
Thanks in advance
#48
its i think against but views has some development going on...
but this patch applies nearly, the rest is rather simple to solve.
#49
The patch definitely needs a reroll, I committed a whole bunch of patchesin the last couple of weeks and it probably broke a few things.
#50
here is a fast rerole
one change, i do not init the plugin on every for of foreach
+ // Ensure we have a localization plugin.+ $this->view->init_localization();
foreach ($options as $key => $value) {
+
if (is_array($value)) {
#51
Hi,
When I apply that patch, I am getting the following error when I try to load Drupal
Fatal error: Call to a member function translate() on a non-object in /home/******/public_html/modules/views/includes/base.inc on line 109
Did I do something wrong?
#52
Hi:
@mrwhizkid: no, you don't make anything wrong, patch #50 contains an small bug.
Also, #50 doens't include the files:
I have rebuild the patch for the last dev version of view 3.x.
To test this patch
Download the last version of views 3.x
cd sites/all/modulescvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d views -r DRUPAL-6--3 contributions/modules/views
Download the patch into views directory and then apply the pacth
cd sites/all/modules/viewspatch -p0 < views-issue-357529-1.patch
@ckng: Please check if the HTML problem is still in this version, I cannot replicate it.
#53
subscribing
#54
subscribing...
#55
Is the goal of this patch to also translate things like the 'text' option (as output by the render_altered method) for fields, or should that be pursued elsewhere?
#56
@jhedstrom:
Can you write a more complete example of what are you talking about? Maybe a screenshot?
#57
Sorry for being unclear. I'm talking about the ability to alter the output for fields. The config options look something like this:
<?php$handler->override_option('fields', array(
'title' => array(
'label' => '',
'alter' => array(
'alter_text' => 1,
'text' => '<em>Highlighted Issue:</em> [title]',
'make_link' => 1,
'path' => 'issues/[title]',
'link_class' => '',
'alt' => '',
'prefix' => '',
'suffix' => '',
'help' => '',
'trim' => 0,
'max_length' => '',
'word_boundary' => 1,
'ellipsis' => 1,
'strip_tags' => 0,
'html' => 0,
),
?>
and the text is altered here (in views_handler_field.inc)
<?php
/**
* Render this field as altered text, from a fieldset set by the user.
*/
function render_altered($tokens) {
// Filter this right away as our substitutions are already sanitized.
$value = filter_xss_admin($this->options['alter']['text']);
$value = strtr($value, $tokens);
return $value;
}
?>
as far as I can tell, this content is never passed through t().
I'm wondering whether this should be fixed as part of this patch, or if it may be as simple as tweaking the render_altered function to pass the text through t().
#58
@jhedstrom:
Yes, you are right. This patch doesn't translate this options. However, I think that views shouldn't take care about this. This is a job for cck module.
+++ includes/base.inc 23 Jul 2009 15:16:27 -0000@@ -85,16 +84,35 @@ class views_object {
+ else if (!$this->view->editing && (!empty($definition[$key]['translatable']) || !empty($definition['contains'][$key]['translatable'])) && !empty($value)) {
+ // Allow other modules to make changes to the string before it's
+ // sent for translation.
+ // Look for a propertyname_format property.
+ $translation_data = $this->view->invoke_translation_process($value, isset($options[$key . '_format']) ? $options[$key . '_format'] : NULL, 'pre');
+ if ($this->view->is_translatable()) {
+ // The $keys array is built from the view name, any localization keys
+ // sent in, and the name of the property being processed.
+ $storage[$key] = $this->view->localization_plugin->translate($value, array_merge(array($this->view->name), $localization_keys, array($key)));
+ }
+ // Otherwise, this is a code-based string, so we can use t().
+ else {
+ $storage[$key] = t($value);
+ }
+ $this->view->invoke_translation_process($value, $translation_data, 'post');
CCK should localize this strings implementing hook_translation_pre_process()
#59
Ooops, this patch doesn't work for "Rewrite the output of this field"
I will try to fix it tomorrow.
#60
Well. Now its simple not working anymore :(
I'm attaching a new patch that fix a bug introduced (in my last rellol) however this patch doesn't solve the real problem.
Today I have been testing this patch and I think that the problem is the implementation of unpack_options()
There is something wrong that make this plugin think that a handler with 'alter' options, is not translatable.
Views is defining correctly this properties in
<?php
function option_definition() {
$options = parent::option_definition();
$options['label'] = array('default' => $this->definition['title'], 'translatable' => TRUE);
$options['alter'] = array(
'contains' => array(
'alter_text' => array('default' => FALSE),
'text' => array('default' => '', 'translatable' => TRUE),
'make_link' => array('default' => FALSE),
'path' => array('default' => '', 'translatable' => TRUE),
'alt' => array('default' => '', 'translatable' => TRUE),
'link_class' => array('default' => ''),
'prefix' => array('default' => '', 'translatable' => TRUE),
'suffix' => array('default' => '', 'translatable' => TRUE),
'target' => array('default' => '', 'translatable' => TRUE),
'trim' => array('default' => FALSE),
'max_length' => array('default' => ''),
'word_boundary' => array('default' => TRUE),
'ellipsis' => array('default' => TRUE),
'strip_tags' => array('default' => FALSE),
'html' => array('default' => FALSE),
),
);
$options['empty'] = array('default' => '', 'translatable' => TRUE);
$options['hide_empty'] = array('default' => FALSE);
$options['empty_zero'] = array('default' => FALSE);
return $options;
}
?>
For this reason I think that maybe is a recursion problem or something like this.
Titles and text for exposed filters are not being translated. Only the title for the view can be translated.
Anyway, patches are wellcome...
#61
Good news.
I have modified the patch and its is working.
Also I did some testing:
Things that are working:
When user clone a view using a language different than English, all the labels are translated, and then, when views is saved, all "english" labels are saved translated. This doesn't occurs if user is using English as language when clone the view. This is fixed in this patch.
All this items are working for override and default displays.
Things that are not working:
* Note:
@ckng in #44:
Finally I can replicate this issue. I have investigate a bit about this issue I found the cause for this problem:
Pascualle said in #352121: locale_string_is_safe() is not suitable for user provided text in non-default textgroups
For this reason, we can't translate headers or footers which contains <div> or similar tags.
I think that views should provide a method to define headers, footers and empty texts translatables like captcha does with the string for captcha challenges.
I'm changing to needs review because HTML headers, footers and empty texts are a different issue.
#62
Thanks for carrying this patch forward.
See #549698: Prepare field label and description for DDT (translatable queries) for a related discussion in core. Possibly we could simplify this patch enormously and move the handler to a general solution that would apply to many non-field string translation needs, not just views.
#63
Subscribing
#64
FYI I've discussed this patch with Earl and it's critical that this patch http://drupal.org/node/503452 is finished (comment #45). Once this is in it will have some larger implications for the localization work here, especially as it applies to exportables.
I'll be focusing my efforts on #503452 though it certainly is quite dense and low level : )
#65
We should consider if and how #593746: Prepare Drupal core for dynamic data translation impacts here.
That issue added query tag based translation to core, with the caveat that, to be translated, a string must be stored in a distinct db field. As many views strings are serialized, we won't be able to use the approach directly.
Could the approach be extended to support serialized data? In the issue, we didn't see how.
#66
Right.
For that "translatable" approach, all fields containing translatable strings (that are rather UI translations, not content translations) would have to be stored in separate table columns and marked as 'translatable' in the schema.
I'd guess that this wouldn't be an issue for the basic views header/footer/empty fields, because I think they apply to all views.
However, it gets a bit messy when considering views field labels, i.e. the caption you can enter for every single field in the view. I guess that this would only be possible if Views would store that views configuration data a bit more denormalized, i.e. so views field labels were stored in a dedicated column.
#67
subscribing
#68
Let's keep this against Drupal 6 for now.
#69
The basic idea works. Committed the i18n side, I hope at least it helps people here testing this part. #360024: Write views localization plugin
#70
Only for reference, I have created a post with some task related to internationalization issues for views #641128: Internationalization tasks for views 3
#71
I've been testing this for a while, looks great overall, but still has some issues:
1. Needs proper handling of texts with format (header, footer, empty). Relying on the implementation of translation_pre_process doesn't look like a safe option, if not implemented for a format all kind of stuff can be passed to the translation system. So at least we should pass the format along with the text to the translation plugin; if it cant handle it that string should be skipped, not translated.
2. When running process_locale_strings() we miss some strings, like field titles. On the other side when running $view->unpack_options() we get that field titles but miss some keys for them: the keys passed are just (view name, 'label').
#72
@Jose Reyero: Mmm, I don't know how do you test this patch into views 3, since it doens't apply, you probably test it into views 2 isn't?
Anyway, here is a reroll for views 3. I made some test, here are my results.
Using Core plugin:
Views title, header, labels for exposed filters and for fields can be translated successfully using Translate Interface UI.
These strings are available to translate once the view is displayed (like in a page) when the alternative language is in use. I don't know if it is a bug, or it is the default behavior for performance reasons. (1)
Using i18n plugin:
Same as Core plugin. As nedjo wrote in the original patch, this method is better because a same translation is used if the original string change. This is really interesting.
I don't understand it. I have created a header using <div> tags whit Filtered HTML input format selected and it is filtered in both, the untranslated string and the translated string. This is the expected behavior isn't?
Mmm, maybe this is related to (1)
Jose, can you explain a bit more what keys are missing?
Note: This patch doens't apply anymore for views 2
#73
@dagmar, I've tested with 3.x (applying some parts of the patch manually)
About texts with format: We need to pass the format along with the text and check format permissions for translators. Otherwise it is not safe, specially if using filters like 'php filter'. (I'm afraid the core translation won't support that, though I'll build support for that into i18nstrings. So we need the format to be passed to the localization plug-in along with the text).
For field labels, I'd expect a keys array like (view name, display id, fields, field name, label) so we can identify each string uniquely without relying on the string content itself.
I'd also expect the function view::save_locale_strings() to save all view's strings so we can really track them down. Currently it just saves some of them (not field labels). Though for core translation it doesn't matter that much, i18nstrings actually tracks/updates/deletes strings when needed so we need a way to get all the strings from a view.
So there's something we really need to address before this can get committed: texts with format.
The other issues you may consider them just improvements.
#74
I've done some update and refactoring of this patch so plugins can really handle text with formats. The patch also needed updating after this one got in #503452: Retool exports to drill down properly
Changes:
1. Full data is passed as an array to all the plugin callbacks. The array has 'value', 'format', 'keys' elements.
2. The translate function now handles this array and there's a 'translate_string()' which is the one to override for simple plug-ins like core localization
2. Moved the pre/post processing function into the plug-in so it can be overriden too.
This is how it looks in the base class (views_plugin_localization)
/*** Translate a string / text with format
*
* The $source parameter is an array with the following elements:
* - value, source string
* - format, input format in case the text has some format to be applied
* - keys. An array of keys to identify the string. Generally constructed from
* view name, display_id, and a property, e.g., 'header'.
*
* @param $source
* Full data for the string to be translated.
*
* @return string
* Translated string / text
*/
function translate($source) {
// Allow other modules to make changes to the string before and after translation
$source['pre_process'] = $this->invoke_translation_process($source, 'pre');
$source['translation'] = $this->translate_string($source['value'], $source['keys']);
$source['post_process'] = $this->invoke_translation_process($source, 'post');
return $source['translation'];
}
I'm not sure about the intention of the pre/post process functions (something with filters?) but this way you'll have a go at the string (and format, and keys) before and after the translation
Now for views_plugin_localization_core we need to override 'translate_string' instead of 'translate'. Same for save() and save_string() methods.
/*** Translate a string.
*
* @param $string
* The string to be translated.
* @param $keys
* An array of keys to identify the string. Generally constructed from
* view name, display_id, and a property, e.g., 'header'.
*/
function translate_string($string, $keys = array()) {
return t($string);
}
Still the issues in my previous comment need to be addressed. But we can move from here if you like these changes.
Note: Also I'll update i18nstrings accordingly but for this one I'll be overriding the 'translate' method because it will handle formats by itself.