Implement translation of customized 'translatable' views properties

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.

subscribing so I can review this later.

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 1566

This 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.

Yes, lets' do additional definition data as a followup patch.

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".

Subscribing

subscribing

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.)

subscribing and testing

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.

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 :)

Thanks, I'll give it a go (^_^)b

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? ;)

subscribe

Thanks for these tips - I was wondering the same.

subscribing.

subscribing

subscribing

Subscribing - should be testing this out in the next week or two

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

The patch doesn't apply for the actual views 2.x

patch -p0 < views-localization-357529-17.patch 
patching 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 :)

subscribe

Yeah guys,

please make this happen!

I've rerolled this patch for DRUPAL-6--3 with the following additions:

• Added export() and export_render() methods to the localization plugin for pushing translatables into exported Views.
• Added export_locale_strings() and the export $op to process_locale_strings()
• Added translatable string export to View exporting. Exportable strings are not processed/evaluated "inline" but are put in a separate $translatables array after each exported view. This allows translation extractors to find translatable strings without affecting the actual translatable stack present in Views.

Couple of small fixes... previous patch had some bad typos : )

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

subscribe

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.

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)) {

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.

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.

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.

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]

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?

adding a tag.

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

its i think against but views has some development going on...

but this patch applies nearly, the rest is rather simple to solve.

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.

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)) {

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?

Hi:

@mrwhizkid: no, you don't make anything wrong, patch #50 contains an small bug.

Also, #50 doens't include the files:

• views_plugin_localization_core.inc
• views_plugin_localization.inc
• views_plugin_localization_none.inc

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/modules
cvs -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/views
patch -p0 < views-issue-357529-1.patch

@ckng: Please check if the HTML problem is still in this version, I cannot replicate it.

subscribing

subscribing...

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?

@jhedstrom:

Can you write a more complete example of what are you talking about? Maybe a screenshot?

Sorry for being unclear. I'm talking about the ability to alter the output for fields. The config options look something like this:

  $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)

  /**
   * 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().

@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()

Ooops, this patch doesn't work for "Rewrite the output of this field"

I will try to fix it tomorrow.

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

  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...

Good news.

I have modified the patch and its is working.

Also I did some testing:

Things that are working:

• Labels for Exposed filters, Fields, etc.
• Text for overriden settings, like "Rewrite the output of this field"
• Headers footers, empty, text *see note below
• Exportables
• Clone views

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:

• HTML headers, footers and empty texts *see note below

* Note:

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)

@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

The block translation is also not part of core. I just say when you want to translate a block it is better to create a new block than using t() on a HTML text blob..

The textgroups should have a format value associated. As format used in views_handler_field_markup in views module. So the strings can be easily checked with check_markup($value, $format, FALSE); at entry also.

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.

Subscribing

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 : )

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.

subscribing

Let's keep this against Drupal 6 for now.

The basic idea works. Committed the i18n side, I hope at least it helps people here testing this part. #360024: Write views localization plugin

Only for reference, I have created a post with some task related to internationalization issues for views #641128: [meta] Internationalization tasks for views 3

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').

@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.

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.

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?

When running process_locale_strings() we miss some strings, like field titles.

Mmm, maybe this is related to (1)

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').

Jose, can you explain a bit more what keys are missing?

Note: This patch doens't apply anymore for views 2

@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.

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.

Upgraded i18nstrings to work with the previous patch. Added support for input formats in texts.

I tried to apply the patch of #74 to the latest views 6x-3.x.dev, but it failed. I use cygwin and I get the following error:

patching file includes/base.inc
Hunk #3 FAILED at 75
Hunk #5 FAILED at 100
2 out of 5 hunks failed -- saving rejects to file includes/base.inc.rej

I am a newbie patcher, so maybe I did something wrong. I saved the patch as a .txt file to the views directory and changed its EOL conversion to windows with Notepad++.

I use the command:
patch -p0 < 357529_views_translatable_full_data.txt

Here is a reroll of the patch.

From #73:

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.

Finally I found the raise of this issue.

The problem is in the function process_locale_strings($op) {} in views.inc, unpack_translatable is only applied to the fields of the display, but fields, filters, arguments etc are not unpacked.

In fact I think this is a important problem, since views doesn't unpack the view after save it, we have to found a method to iterate on each object to save the strings.

Core translation works because t() storage the string when is called, and this function is called when unpack_options is called for every handler (when the view is loaded)

So, Ideas to solve this? we can implement a totally different unpack_translatable function but since core translation works, maybe the best way to implement this is the i18n plugin. I don't know.

Patching went smoothly and I went straight into testing.

Where translation was successful:
1) Created a node type View with page display and field Node: Nid
2) Added Title, Header, Footer
3) Searched text entered at http://www.example.com/admin/build/translate/search and instantly found the strings to translate. Worked Great!

Question: When I go back to the View and change the title, the old title is still in the translation system even after refreshing all strings. Is this as per design? I would like to confirm with you so that I will know to go and manually delete in the future.

Where translation was unsuccessful:
1) Created a node type View with page display and field Node: Nid
2) Checked Rewrite the output of this field and typed text: View Me
3) Saved the view, refreshed translation strings and searched at http://www.example.com/admin/build/translate/search

I was unable to find the 'View Me' string (I made sure search was case-sensitive).
I attempted refreshing again at http://www.example.com/admin/build/translate/refresh, and flushing all caches.

Please let me know if further information is needed and if you would like testing of other parts of views (fields and labels translated great as well, but not sure if that is views or cck).
Thanks for the great work!

Is translation of 'Rewrite the output of this field' still not included?
I saw at http://drupal.org/node/555546#comment-2301136 that it will be implemented here.

Patching with #77 was good. Thanks

@rburgundy - dagmar helped me with this. Here are the instructions:
1) Save the view.
2) Change to the language you want the field translated to
3) Display the view using this language.
4) Go to Administer -> Site Building -> Translate interface and search for the field

I look forward to further development in implementing translations in Views.
Thank you

Hi,

Just jumping in, after my recent blog post about this, where dagmar asked if I might come here and explain my comments about Views and Menu. (See this comment.)

The reason I made the comment is it seems the i18n Views translation support picks up the menu text once from the View and then the i18n Menu support picks it up again from the created core Menu module item, so things get messy. It's not too big a deal, but as a consequence we stopped adding menu items via the Views interface, as having two strings to translate for each one item was a bit of a pain. And it was just easier and less confusing to simply use Menu and forget Views has a menu item feature.

I'm not sure what the fix would be. It probably actually isn't a fix to Views that is required, but more a fix to i18n Views support to ignore the menu elements of a View if there is already a core Menu entry corresponding, maybe.

Anyway, thought I'd explain the problem, as requested. Hope it helps and look forwards to trying Views 3. =)

Question: When I go back to the View and change the title, the old title is still in the translation system even after refreshing all strings. Is this as per design? I would like to confirm with you so that I will know to go and manually delete in the future.

Thats the problem of using the t-method.

thanks dereine, I will try to find a way to clean-up strings that are not used.

I hope this tag is ok.

So here is a new version.

The handlers does get unpacked, too. You can look at the patch, or alternative see the commits in http://github.com/dereine/drupal-views/commits/i81n

What does currently needs some work:
- The export does not export every option

I'm wondering why the export displays t() for every export item. Shouldn't the export plugin be possible to define it's own translation method?

@dagmar
It would be cool if you would look at http://github.com/dereine/drupal-views/commit/776fc6627db008e74ce3e87db8... and http://github.com/dereine/drupal-views/commit/bf004d5a34d068add2632dceeb...

So here is a new version, which makes export working also for handler-strings
This fix was trivial: http://github.com/dereine/drupal-views/commit/17075f103882bf480a7433df90...

I think now its time to get some reviews.

@i18n
Perhaps your plugin should also implement the export and export_render method.

We need a reroll for this patch, it doesn't apply anymore.

Here is a fast rerole

A few things to make this patch easier to test.

As I said in #78

Core translation works because t() storage the string when is called, and this function is called when unpack_options is called for every handler (when the view is loaded)

We should do a more exahustive test using i18n (Views translation) than core method. To do this, we need a reroll of the patch for i18n to make test with this patch.

Anyway, I test this patch "as is" and there is some thing that needs to be fixed.

1) It seems, $edit variable is not working correctly:

When a view is created, the strings defined in it, like Title, or the labels for fields, have to be defined in English, and then, translate them using the Translate interface UI. In my test, when I edit a view using English as language, the string are displayed in English (in the edit UI of the view), but when I edit the same view using another language, like Spanish, the strings are translated and displayed in Spanish. The problem of this, if is I save the view in Spanish, I lost the original string. So the view should display the strings "always" in English (or technically speaking, in the original language that were written, and don't change between sites with different languages.)

2) When a view is exported, the translatable part doesn't include all the available items. i.e, All the strings for exposed_form_plugin are missing.

3) Exposed forms plugins allows users to define their own custom string for apply, reset, order by, etc. At this moment, they are translated using t(). This calls to t() in views_plugin_exposed_form.inc should be replaced by check_plain()

4) Merlinofchaos said in the IRC:

dagmar1: We need to provide a patch to potx that will allow it to identify strings from default views and make them available for translation.

5) As personal note. I think we should provide a bit more of information in the edit UI to inform users how to translate their strings. Also we still need to define a method to translate "Rewrite the output of this field" and things like that.

subscribing

Subscribing...

Subscribing. Thanks for all the hard work on this.

Subscribing.

About i18n, moved i18nviews here so we can have a branch for each views version (3.x branch includes the views translation plug-in)
http://drupal.org/project/i18nviews

sub

Update tag.

It looks like another awesome bit of drupal. I have trouble understanding what to do to make it work or even to test it at the moment. Am i correct in assuming that I need to patch views AND install i18nviews right now? So far I switched to views-6.x-3.0-alpha3 and enabled i18nviews, but see no difference. Thanks for any help.

You need to apply the patch above.

+1

This is different.

This uses a structure which is very similar to the 3.x export machinsm

The patch has 0 bytes.

verifying #101 - there's no content in the patch in #100.

I suck at this time.

So here is a patch

This still does not tacle the text area problem.

If someone want to write a patch feel free to do it :)

Here is a new patch, but i'm not sure what was changed :)

This patch definitive differs from the patch before :)

Oh orig and rej files.

Subscribing, greetings, Martijn

Ok, here's my commentary:

There's a stray dsm() in there.

I don't see where ->translate() method is actually called. This is making it difficult for me to understand when and how translations are actually to be used. I recall from older versions of this patch that if a view was being loaded for the purpose of being edited then it was supposed to not translate on load, but otherwise it was supposed to? Furthermore,_set_option_defaults() is still calling directly to t(). My guess is that something has been lost in here somewhere?

I don't fully understand unpack_translatable() or why it's separate from unpack_options(). at the very least this needs comments to explain what is going on here. Maybe I don't understand it becomes of the previous paragraph.

Should the export_render() that's on the 'core' localization plugin actually be default? Scenario: I am developing a module. I have translation off for efficiency. When I export to code, I still want my translatable strings to be marked with the t() function.

export_render() should enclose its translations in an if(0) -- Unless I'm mistaking, we're marking those strings for the use of potx, right? AS it is now, every time default views are loaded, all of those strings will be run through t() -- even if you're using a different translation method. If you've got 30 exported views in your system and each one has an average of 20 translatable strings, then that's 500 useless t() calls during those operations. That seems bad.

The base localization plugin should have a var $translate = TRUE. The 'none' should have var $translate = FALSE. Then init_localization() should return plugin->translate after it successfully initialize the plugin. Why? If translation is off, the plugin should never be called. We can avoid calling into the functions and skip several hook calls that are unnecessary for cheap.

Finally, it seems like the bulk of view::process_locale_strings() should actually be on the translation plugin, and probably on the default implementation. While I can't think of scenarios where a plugin will want to change that code, it would at least keep the code, as much as possible, with the translation plugin where it belongs.

In fact, comparing the last patch to #88 I see some of the code that was lost.

Ok, I looked at #88 to see the missing code. Further commentary:

I don't understand the 'core' save_string() method. What is this doing and why? Nothing in Views calls 'save' so I wouldn't think it would be utilized?

I don't like the code in for is_translatable() -- maybe for 'default' views we should automatically assume that the translation plugin is 'core', regardless of what the sitewide plugin is. Additionally, maybe we should do the same thing when exporting so that exports are consistent no matter what translation plugin you have active.

Yeah, add Nedjo's comments in #24 to the TODO list generated by my review.

so

subscribe

I don't understand the 'core' save_string() method. What is this doing and why? Nothing in Views calls 'save' so I wouldn't think it would be utilized?

This code calls save_string:

  function save($source) {
    // Allow other modules to make changes to the string before saving
    $source['pre_process'] = $this->invoke_translation_process($source, 'pre');
    $this->save_string($source['value'], $source['keys']);
  }

I don't like the code in for is_translatable() -- maybe for 'default' views we should automatically assume that the translation plugin is 'core', regardless of what the sitewide plugin is. Perhaps i'm just wrong here.

Mh. Let's take the example openatrium. They quite sure they want to have i18n as translation plugin

Right. I said nothing in Views calls 'save'. 'save' calls save_string, but nothing calls 'save'.

As for openatrium wanting to export as i18n, that suggests we might need a separate control for exporting views as well.

oh yes.

Is this the place to follow for the translation of the views Title?

I go to /admin/build/views/edit/search and set the Title in English.
Then I go to /fr/admin/build/views/edit/search and set the Title in French.
But the title for both English and French are now the French Title last inputted.

Thank you

The base localization plugin should have a var $translate = TRUE. The 'none' should have var $translate = FALSE. Then init_localization() should return plugin->translate after it successfully initialize the plugin. Why? If translation is off, the plugin should never be called. We can avoid calling into the functions and skip several hook calls that are unnecessary for cheap.

Fixed this part.

Finally, it seems like the bulk of view::process_locale_strings() should actually be on the translation plugin, and probably on the default implementation. While I can't think of scenarios where a plugin will want to change that code, it would at least keep the code, as much as possible, with the translation plugin where it belongs.

Adressed.

export_render() should enclose its translations in an if(0) -- Unless I'm mistaking, we're marking those strings for the use of potx, right? AS it is now, every time default views are loaded, all of those strings will be run through t() -- even if you're using a different translation method. If you've got 30 exported views in your system and each one has an average of 20 translatable strings, then that's 500 useless t() calls during those operations. That seems bad.

So no t() should be used there. Perhaps a comment like "Start translatable strings" which can be scanned. (not adressed yet).

I don't fully understand unpack_translatable() or why it's separate from unpack_options(). at the very least this needs comments to explain what is going on here. Maybe I don't understand it becomes of the previous paragraph.

This method was introduced to be runned on saving the view to call save() on the strings. See #357529-77: Implement translation of customized 'translatable' views properties.

Furthermore,_set_option_defaults() is still calling directly to t(). My guess is that something has been lost in here somewhere?

Will this not be removed later?

Unpack_options also don't call localization_plugin::translate.
I merged the old code in.

Regarding #110 I hope nedjo can answer this questions.

For all people, please use #88 unless you want to bring this patch forward.
The problem with #88 is that it does not work for anything in handlers.

I tryed to bring missing code from http://drupal.org/files/issues/357529_views_translatable_full_data.patch into it.

Some progress later.

Here is the github link if someone want's to help or just look at it: http://github.com/dereine/drupal-views/tree/357529-new

* Fixed export
* Regarding #24

  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'];
  }

and source is

          $translation_data = array(
            'value' => $value,
            'format' => isset($options[$key . '_format']) ? $options[$key . '_format'] : NULL,
            'keys' => array_merge(array($this->view->name), $localization_keys, array($key)),
          );
          $storage[$key] = $this->view->localization_plugin->translate($translation_data);

So the translation plugin can add something here.
Perhaps i'm not sure whether this can fix everything what's needed here.

After applying the patch from #120 to the latest dev version of Views, I keep getting an error whenever I click on the "Update" button while editing a view:

Error Description:
Fatal error: Call to a member function init_localization() on a non-object in .../views/includes/base.inc on line 88

I will upload the latest patch but you should just use this patch at the end if you are able to fix them for yourself

a bit of a summary

* #24 seems to be solved with the comment in #120 right?
* Some answers of merlinofchaos should be answered by nedjo
* Fixed

Fatal error: Call to a member function init_localization() on a non-object in .../views/includes/base.inc on line 88

with this version of the patch.

i will work the full next weekend on views so if anyone is interested especially in this issue join #d7sc during the weekend.

Dereine, sorry I didn't understand your post at #122. I have no idea how Views' code works, so I can only help with testing, and willing to do so.

I applied the latest patch, couldn't see any difference, the issue is still there. It happens on every view and on every field/argument/filter that i try to update.

Fatal error: Call to a member function init_localization() on a non-object in .../views/includes/base.inc on line 88

There is a reason why this patch has the state "needs work" :)=

I get it, I just saw in #123 that you are saying

* Fixed

Fatal error: Call to a member function init_localization() on a non-object in .../views/includes/base.inc on line 88
with this version of the patch.

and looks like it actually isn't

oh Thanks!

So. The tests showed that export of handler labels didn't worked. Now this values get's exported, too.

@zualas
Fix this issue

Updated the patch:

* Don't translate the labels of the export form buttons via t()

Update again, this was the wrong patch

Dereine, your patch is translating my blocks titles and pages generated with Views 3.x dev, that's great, thanks.

I tried to apply the patch from: #77 and it seems that there are some inconsistencies.

You can find the screenshot attached.

Which exact version of Views 3.x-dev is this made against? I am getting a lot of rejects on the latest one (Oct 17).

Mh it should be always the current.

* Test: all strings get's translated
* Test: strings don't get translated in the ui.
* (existing) Test: all strings get's exported

Based on 2) a fix is added

Ok, I applied the patch successfully and got some strings in the Views category. But should this patch also gather (exposed) filter labels as well? I don't see them, but maybe I am missing something.

@memfis

Do you want to extend the test? ;)

I tried extending the test, but it did not fail...

Here's what I added to view_unpack_translatable (plus 'simple2' to expected strings):

    /* Filter: Node: Nid */
    $handler->display->display_options['filters']['nid']['id'] = 'nid';
    $handler->display->display_options['filters']['nid']['table'] = 'node';
    $handler->display->display_options['filters']['nid']['field'] = 'nid';
    $handler->display->display_options['filters']['nid']['exposed'] = TRUE;
    $handler->display->display_options['filters']['nid']['expose']['operator'] = 'nid_op';
    $handler->display->display_options['filters']['nid']['expose']['label'] = 'simple2';
    $handler->display->display_options['filters']['nid']['expose']['identifier'] = 'nid';
    $handler->display->display_options['filters']['nid']['expose']['single'] = 0;
    $handler->display->display_options['filters']['nid']['expose']['reduce'] = 0;

But the real issue I think is that unpack_translatable does seem to find filter labels, but the keys are generated badly, which results in each string being overwritten by the other in i18nstrings. I tried changing the logic that builds $translation_keys in base.inc, but whatever I tried I couldn't get it to include the field or filter name. And even if I would succeed I would still need to copy this logic into the place where translations are retrieved - and I don't really know where that is.

Could you help with this issue?

Ok, I found what is happening: when unpack_translatable invokes $this->{$definition['unpack_translatable']} that goes into unpack_handler, which does not pass $parents nor $keys and the "path" gets lost. It is possible to fix this with unpack_translatable and unpack_handler, but not so much in unpack_options, which does the actual translation and also loses the "path" when invoked from various places in Views. Perhaps instead of plugging into unpack_options, the translation could occur in a dedicated methods invoked before the view is rendered? What do you think? Or did I fail to understand the rationale behind the current approach?

@memfis

Sorry for forgot to answer your email adress. This week was a bit crazy.

Here is the same patch with the extended simpletest.

@memfis
Which version of i18nviews do you use?

I'm using Internationalization Views 6.x-3.x-dev (2010-Jul-21).

I ended up doing the translation through theme_preprocess_exposed_form and replacing labels from there.

I think it's time to get this in and let people report on what's broken about it.

I made a couple of changes:

1) Many notices warnings during export due to some broken code.
2) Keys were not complete. This MIGHT impact i18nviews because now there are much longer keys.
3) style translation unpack was failing completely because styles aren't identified the way other plugins are due to the way the design moved forward.
4) A couple of other minor notice fixes.

actual patch I committed is attached. It almost applies to D7, there's only a couple of fails.

There are some @todos that need to be addressed. Let's do followups in separate issues:

1) I think 'export' needs its own choice for translation plugin to use, because it's entirely possible ot have a site with translation where you need to export in the standard t() manner for inclusion in a module, but you may also want to export in the standard i18n manner for personal use. I think this needs to somehow be selectable.

2) It seems like we could at least provide a code comment on the key arrays in the core export method. Even if it's just views_var_export($keys) inside /* */ so that you can see what the translated string belongs to.

Ported to d7.

Let's open new issues for upcoming issues/the todos etc.

I'm using the last views-6.x-3.x-dev and last i18nviews-6.x-3.x without any patch. I translated my views header in translate interface(views section), but when my views page is displayed, the translation of my views header is not handled(didn't try with footer). It was working correctly in 2.x.

PS: my nodes(title/body) and taxonomy term/description are correctly translated in views in display page and views in panels.

See #143