Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making piwik translation ready #5482

Closed
mattab opened this issue Jan 4, 2008 · 44 comments
Closed

Making piwik translation ready #5482

mattab opened this issue Jan 4, 2008 · 44 comments
Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Jan 4, 2008

There is still work to make piwik fully translatable.

We have to all strings
- in templates
- in JS variables
- design a generic system for the php exceptions to handle multilingual error messages
Keywords: i18n translation translate internationalization

@mattab
Copy link
Member Author

mattab commented Feb 18, 2008

Translate also
- columns labels

@mattab
Copy link
Member Author

mattab commented Mar 19, 2008

(In 390) – fix #5563
- refs #5482: started work, prepared Piwik_Translate, the smarty modifier, translated the Login plugin in french & english as a use case
- fixed redirection bug in safari

@zawadzinski
Copy link
Contributor

Javascripts translations can be done by renaming [to [filename](filename].js)_js.tpl and putting in those smarty templates translations strings (I can include this change in my template translation patch which is nearly done).

Theres are problems with:
- menu strings
- database fields translations (we can use some general function that will be used to translate column name from database to string, even english version can benefit from it)
- dates and periods (e.g. in Home module)
- plugin descriptions (cannot be translated now, because loading translation files for plugins use the function that returns description)

@zawadzinski
Copy link
Contributor

Submitted patch introduces translation strings in templates.

Some things to discusss:
- should all plugins translations be in seperate plugins/[pluginName]/lang/*.php files?
- moving javascripts to smarty templates
- translating database columns to strings, maybe Piwik_TranslateColumn(table_name, column_name)?
- menu translation schema in $translations array, maybe use ‘Menu_$mainMenuName_$subMenuName’ (and ‘AdminMenu_$menuName’), than translate menus automatically in Piwik_GetMenu() / Piwik_GetAdminMenu()

@zawadzinski
Copy link
Contributor

Attachment: Merged changes with current (409) revision
[i18n.patch](http://issues.piwik.org/attachments/5482/i18n.patch)

@zawadzinski
Copy link
Contributor

Attachment: i18n.patch + some example approach for JS translation (see more in comment)
[i18n_withJs.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.patch)

@zawadzinski
Copy link
Contributor

Javascript translations.

If you use text strings in your plugin:

1) put in Controller:
$view→loadTranslation(‘General’);
$view→loadTranslation(‘yourPluginName’);

2) whenever you use string make it translate-friendly:

```
(’yourPluginNamestringIdentificator’,‘default string/message’)

you can also use some popular translations from ‘General’ module:

(’GeneralBack’,‘back’)
```

3) in php translation files define your variable keys as:

```
‘yourPluginName_stringIdentificator_js’ => ‘string/message translation’

so the translation team will know which strings should be translated
```

Example of working code is in SitesManager plugin.

@mattab
Copy link
Member Author

mattab commented Mar 25, 2008

quick remarks:
- All strings that include number, ie. “13 visitors yesterday” should appear as “%s visitors yesterday” in the config file.
Currently:

```
{$nbVisitors} {’General_yesterday’|translate}
```

Should be

```
{’General_yesterday’|translate:$nbVisitors}
```

ie. we give the value to be replaced as an argument of the smarty filter.

NB: When there are more than one value to be sprintf (should be very rare) we should number the %s, ie

```
$format = ‘The %2$s contains %1$d monkeys’;
```

Because in different languages numbers orders can be swapped in the sentence.
- I’ve noticed that in rare cases the coding standard is not followed, for example in getJsTranslation()
– there is no comment at the function
– the

```

if( !$moduleList ) return ’’;

```

should be

```

if( !$moduleList ) { return ’’; }

```
- your idea of the _ javascript function seems good to me. However this name is not good as there will be conflicts with other javascript codes. Think that piwik HTML and JS code will be very often included within other CMS / blogs / etc. so we should make sure the function/class names don’t conflict with other names. How can we handle this problem?
- I can’t see in the code where you use the smarty variable $moduleTranslations built in the loadTranslation(); havent you forgotten to add the new smarty filter you created?
- this loadTranslation() function could have a better name like loadJavascriptTranslation() or something else?
- maybe the call to loadTranslation() could be automatic in some ways? For example instead of assigning a smarty variable, maybe we could call a smarty function from the template that would load the _js prefixed strings? Other idea?

anyway this is good work in progress :) i hope this review is useful.
good luck!

@zawadzinski
Copy link
Contributor

Attachment: new version of i18n patch with javascript translation functions
[i18n_withJs.2.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.2.patch)

@zawadzinski
Copy link
Contributor

I made several modifications,
- now we use smarty function for loading translations, i.e:

```
{loadJavascriptTranslations modules=‘SitesManager General’}
```
- `()` function was renamed to `_pktranslate()`, if there are more general functions I think it will be good to make a javascript namespace with all of them, i.e. i think we need sprintf function for some of javascript translations… and there will be certainly more of them
- coding standards are now followed more strictly, but feel free to find those places where it is not :)
- I added translation files which I forgot to `svn add`. There is quite a lot of small files to translate now, and I think it can be good to put all ‘standard’ plugins’ translation strings into one file to make translators’ work easier…

ps. I couldn’t find any translation that was done without format strings, but maybe I missed sth?

@zawadzinski
Copy link
Contributor

Attachment: current i18n patch (compatible with rev 424)
[i18n_withJs.3.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.3.patch)

@mattab
Copy link
Member Author

mattab commented Apr 1, 2008

- +{literal}Last modification time, description and reporter are included as hidden fields for useful RSS export.

why this add?

- <span><strong>{$nbUniqVisitors}</strong> {’VisitsSummary_NbUniqueVisitors’|translate}
should be
<span>{’VisitsSummary_NbUniqueVisitors’|translate:“<strong>”.$nbUniqVisitors.“</strong>”}
- btw there shouldn’t be HTML in translations, except for the <br>
- your concept of _pk_translate seems quite good, because it makes it easy when there are no translations.
- the commenting on the smarty plugin could include an example of usage it would be more easy to understand
- good job so far. we have to think also about how to make it easy for other people to translate piwik, we need a script that reads all the strings in all plugins, requests user translations in a nice HTML form, saves files in the given plugins. What do you think about it? We can have a look at how Facebook translations work.

PS: sorry for the delay now I should be able to review future code much faster :)

@zawadzinski
Copy link
Contributor

Attachment: compatible with rev 432
[i18n_withJs.4.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.4.patch)

@zawadzinski
Copy link
Contributor

New patch:
- fixed bug in translate smarty modifier (now it supports more than 1 argument)
- fixed translation strings with numbers (i.e. %s unique visits)
- included sprintf() javascript function to the project
- support printf() arguments in _pk_translate
- added documentation for loadJavascriptTranslations smarty modifier
- added js translation strings in plugins

PS. somehow you managed to break trac html filter (everything in bold now) ;>

@mattab
Copy link
Member Author

mattab commented Apr 3, 2008

it seems you haven’t added the right patch (the patch is still the same).

whats needs to be done in order to have a working i18n version of piwik?

we have to think of the Plugin that we have to write to make it easy to translate piwik, through a simple web interface that would
- make it easy to translate all strings in one page
- generate the plugin translation files automatically ; or no generation if there is no string provided ; doesn’t generate the non translated strings in the files (it will defaut to english strings anyway)

@mattab
Copy link
Member Author

mattab commented Apr 3, 2008

NB: we have to make sure that all exceptions thrown before the language files are loaded are ONLY thrown in ENGLISH and that the text is hardcoded in the source files.

—> how to make sure all these exceptions are hardcoded?

@zawadzinski
Copy link
Contributor

Attachment: right patch
[i18n_withJs.5.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.5.patch)

@zawadzinski
Copy link
Contributor

- I suggest creating seperate ticket for the translation plugin
- my idea for exceptions is:
– extend Exception class so we can pass 2 arguments throwing an Exception
– first one is translation string, second english message i.e.

```
throw new PiwikException(“General_PluginNotFound”,“Plugin not found”);
```
- in Exception handler we check`
functions_exists(“Piwik_Translate”)`
– if it does exist, and there is a translation we display translated string
- if it does not, we display default english message

PS. I attached the right patch :)

@mattab
Copy link
Member Author

mattab commented Apr 3, 2008

‘’’ \* agree, please create a new ticket with your specification
- for exception there is no point trying to translate the few exceptions that would occur before the translations files are loaded. Everything after the call to`
Piwik::loadPlugins()` (line 305 in /trunk/modules/FrontController.php) can be translated, but the few exceptions before this line cannot. I would suggest to go for Exception(‘Message in english’) and Exception(‘IndexTranslation’) : both of them would work. ie, if the dev doesnt care about translations, it’s ok it stays the same simple syntax. If he cares, then he puts in an index and he has to add the entry in the english language file. The class name would be`
Piwik_Exception`
- the problem in the current architecture I can see is that we have to duplicate the JS strings, which is annoying. Is there any other solution you can think of?
- other than that your changes are good :)
- can you think of other things to be done in order to have a proper i18n support? I can think of
– “right to left” language support
– proper date translation handling
– table column labels

@zawadzinski
Copy link
Contributor

Just several quick comments and issues to discuss.
1. Exceptions.

Message translation cannot be done in Piwik_Exception class, because getMessage() method is not overridable. I will check for translations in Piwik_ExceptionHandler.
1. Javascript strings duplication.

We certainly need definitions in translation files, so the only way is not to put default string in js code, and by default load english strings.
1. Column labels.

I haven’t understood all of datatables code yet, but we can call _pk_translate() in javascripts on column labels (and somewhere store original labels to keep working sort functions etc.). The same applies to other chart labels, but I need some help with it in order not to waste much time on it :)

@zawadzinski
Copy link
Contributor

Attachment: TranslateException, translation strings for exceptions in plugins/
[i18n_withJs.6.patch](http://issues.piwik.org/attachments/5482/i18n_withJs.6.patch)

@zawadzinski
Copy link
Contributor

- new function Piwik_TranslateException used to translate exception messages – neither overriden class nor translation in Piwik_Exception (log/Exception etc.) can be done, because than we cannot use sprintf for translations with parameters.
- added string translation for plugins

TODO:
- add exception translation strings for core piwik
- column labels
- dates
- “right to left” language support

@zawadzinski
Copy link
Contributor

Attachment: solves also #65 and #116
[i18n_feedback_password.patch](http://issues.piwik.org/attachments/5482/i18n_feedback_password.patch)

@zawadzinski
Copy link
Contributor

Some quick remarks on last patch:
- Zend_Mail, Zend_Mime added,
- Piwik_Mail class derived from Zend_Mail
- fixed problem with thickbox colliding with submenus
- plugins/Login – remind password feature and some fixes to store properly first request URL
- plugins/Feedback – new plugin to send feedback to Piwik Team
- Piwik_Common – new function to generate random strings from given alphabet
- all latest i18n fixes

@zawadzinski
Copy link
Contributor

Idea for columns translations interface:

in Controller.php add

```
$view = Piwik_ViewDataTable::factory();
(…)
$view→setColumnsToDisplay( array(‘label’,‘nb_visits’) );
$view→setColumnTranslation(‘label’, ‘Referers_ColumnLabel’);
$view→setColumnTranslation(‘label’, ‘General_ColumnNbVisits’);
```

or maybe even better assigning translated string:

```
$view→setColumnTranslation(‘label’, Piwik_Translate(‘General_ColumnNbVisits’));
```

Then change modules/ViewDataTable so they pass translated strings values to templates (if translation does exist, in other case translated string = column name).

And change templates so instead of column names display translated labels.

I tried to find a place where it can break widgets (e.g. sorting, paging) but didn’t find anything… maybe I haven’t noticed sth?

@zawadzinski
Copy link
Contributor

Attachment:
[i18n_feedback_password.2.patch](http://issues.piwik.org/attachments/5482/i18n_feedback_password.2.patch)

@zawadzinski
Copy link
Contributor

- added column translations
- fixed bug in _pk_translate()

@zawadzinski
Copy link
Contributor

Attachment:
[i18n_feedback_password.3.patch](http://issues.piwik.org/attachments/5482/i18n_feedback_password.3.patch)

@zawadzinski
Copy link
Contributor

CHANGES:
- setlocale(LC_TIME, …) in main translation file in order to support strftime
- new Piwik_Date→getLocalzied() method
- i18n of calendar & period chooser

TODO:
- menu names
- plugin descriptions
- graph labels (i.e. number of pages, visit duration in Frequency & Loyalty)

@zawadzinski
Copy link
Contributor

Attachment:
[i18n_feedback_password.4.patch](http://issues.piwik.org/attachments/5482/i18n_feedback_password.4.patch)

@zawadzinski
Copy link
Contributor

Changes:
- added method postLoad() for plugins which is called after loading plugin and corresponding translations
- added submenu & widget submenu label translations

TODO:
- main menu labels (need to rewrite some of menu code)
- plugin descriptions
- graph labels (i.e. number of pages, visit duration in Frequency & Loyalty)

@mattab
Copy link
Member Author

mattab commented Apr 17, 2008

(In [[mauser](447]))- refs #5482 commited huge work by Maciej Zawadzi?\197?\132ski: thanks! still some work to do + QA but looks really good :)
[mauser]- refs #5536 commited partial fix
[mauser]- refs #5485 commited partial fix
- fixed broken tests

@mattab
Copy link
Member Author

mattab commented Apr 17, 2008

feedback on the submitted patch
- your choices for exceptions / columns label are good
- I can see a problem for the current javascript strings. The fact that the strings are duplicated is not reliable. Above #5482 I think that to load by default the english string is a good idea.

``

`
string = pk_translate(’HomeIncludeAllPopulation’);

would be better than the current
``````

string = pk_translate(’HomeIncludeAllPopulation’,‘Include all population’);```

And if the dev doesn’t care about translations he would simply use hardcoded strings in his JS files…
- I am still not sure about how to do the menu translation
- I suggest we don’t translate plugins descriptions. I have never seen a software that translates the plugin descriptions. I think this is not really useful.
- graph label translations: do not handle it at the moment, as the whole graph labelling architecture is not here yet, the labelling is often wrong (always nb_visit). We will address this problem later.
- great job! this is not easy and requires hard and cautious work, this is an amazing new feature to piwik…

@anonymous-matomo-user
Copy link

Status of german translation is nearly 80%. (i would note it here, so that nobody else started to translate) :)

@zawadzinski
Copy link
Contributor

Attachment: _pk_translation without default strings, fix in Visitors→Overview view
[js.patch](http://issues.piwik.org/attachments/5482/js.patch)

@zawadzinski
Copy link
Contributor

small patch
- _pk_translation is now without default strings
- fix in Visitors→Overview view

@anonymous-matomo-user
Copy link

German languages files for avaiable plugins attached – pls integrate into svn.

@anonymous-matomo-user
Copy link

Attachment: german language files
[de.patch](http://issues.piwik.org/attachments/5482/de.patch)

@zawadzinski
Copy link
Contributor

Attachment: fixes #62, #65, #116 and adds german translations
[multi.patch](http://issues.piwik.org/attachments/5482/multi.patch)

@mattab
Copy link
Member Author

mattab commented Apr 24, 2008

(In [[mauser](454])) – fixes #5536, font consistent, escape key now works
[mauser] – fixes #5485, now catching error, printing friendly message, refactoring template code
[mauser] – fixes #5482, fixing JS translations api, Piwik is now ready to be translated!!! :)

@mattab
Copy link
Member Author

mattab commented May 5, 2008

We must add the information of the translators in the main translation files.
- name
- email
- language label in english (‘english’, ‘polish’, ‘french’)
- language label in original language (‘anglais’, ‘polski’, ‘franais’)

@zawadzinski
Copy link
Contributor

Support for right-to-left languages:
- add conditional css style that will overload text classes/ids
– direction: rtl
– text-align: right (when original is left)
- add definition in translation file indicating what kind of langauge we load (rtl,ltr)

@zawadzinski
Copy link
Contributor

- Example of a custom RTL CSS
– http://trac.dojotoolkit.org/browser/dijit/trunk/themes/tundra/tundra_rtl.css?rev=11656

@mattab
Copy link
Member Author

mattab commented Jun 9, 2008

Please, let’s leave this ticket, the first part of the plan has been implemented.
We are now working on part 2, see #5671

@mattab mattab added this to the Piwik 0.9.9 - Stable release milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

3 participants