You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 19 Next »

Rationale

Magnolia 5.0 went slightly backwards - we lost some translations. The mechanisms are still in place, but we "forgot" to implement them; many dialogs are configured with "hardcoded" labels. Some code, too.

More importantly, the way we apply i18n through the UI is currently inconsistent or inexistent. Most new constructs (apps, action bars, ...) have no notion of i18n. Blindly applying the same slightly dated concept we've been using up to 4.5 would blow the configuration out of proportion. Below are a few suggestions to explore and make this simpler, smaller, or more consistent.

Topics left to explore

  • (Updtask remove if match)
  • (Check contents)
  • Migrate existing translations
  • Chain bundles for simple defaults (chain with generated bundle names, sort of package fallback)
  • Process for contributing and integrating contributed translations
  • Multi/composite fields

Existing uses and inconsistencies

info.magnolia.ui.form.AbstractFormItem#getMessages - should not be public

info.magnolia.ui.form.AbstractFormItem#getMessage - should not be public

AbstractFormItem defines a semi-arbitrary message bundles chain (see AbstractFormItem#UI_BASENAMES)

Definition objects( TabDefinition, etc) have an i18nBasename property, which is very redundant with that of the "runtimes" objects ( FormTab, ...). Usage seems consistent ( return definition.getI18nBasename();) but I don't know why this isn't implemented in info.magnolia.ui.form.AbstractFormItem.

Definition objects don't have a common interface. If they did, we could move i18nBasename and label in there. OTOH, some of these objects have more than 1 item to translate (label and description, for example).

view.addFormSection(tab.getMessage(tabDefinition.getLabel()), tab.getContainer()); ...translates the message from the tab and passes it translated before it's actually "displayed". (while the method argument is called tabName not tabLabel - but that passed object becomes an argument called caption later down the stack) - the below would make this sort of code much more explicit. You pass an object meant to be a label. You translate it explicitly - most likely at the last possible moment. Or we even extend Vaadin component so that they know about I18nItem.

 

Food for thought: 

Change info.magnolia.ui.form.field.definition.FieldDefinition#getLabel. Node2bean should be made aware of I18nItem class (by registering a transformer)

change this
String getLabel();

into this

into this
 I18nItem getLabel();
  • N2B would instanciate these
  • it should be made aware of its "parent" (FieldDefinition etc)
  • via injection, the impl would know what locale to use and delegate to MessagesManager (or its replacement)
  • we'd have a default null-pattern type of implementation (which returns the key or an empty string, or sthg else - this could even be swapped depending on dev mode etc).
  • Would this impact performance/memory usage ? 
  • Quick prototype of this attached - doesn't do anything, and doesn't know about Locales or message bundles - just showing the "structural" change on classes like FieldDefinition (patch file - disregard class names and packages !)

Ideas and topics discussed on 2013-08-06

  • Keys deduction is ok
  • N2B will not work
    • can't really get the parent into the object (children objects are instanciated first) - not without modifying usage code (ie in setLabel() { label.setParent(this) }, which would suck)
    • can't really transform single properties other than with beanutils, which has even less notion of context, and whose usage is currently hardcoded in core 
  • Proxy definition objects ?
    • I18nText will need some sort of context, the definition objects are not enough, since they don't know about user language etc.
  • Basename could also be defined in site definitions (for in-template translations)
  • Bundle languages in separate jars
    • english/master language still bundled with each module, other languages bundled in 1 jar (1 jar per language, containing translations for many modules)
    • maintenance is somewhat easier
    • but at the same time we might get "dependency" issues when modules add/remove keys
    • if we have tools for migration/validation of existing translations, the same tools could be used, perhaps as a maven plugin or sthg.
      • such a tool could potentially help enforcing compatibility between versions (i.e keep a key that was removed in version X+1 of module M)
  • Ideas of supporting things like some.key=${some.other.key
    • mention of the CZ issue where in english a translation would be the same in 10 place but needs to be adapted in czech.
      • somewhat moderate issue with the mechanism is only supported with the same file
    • sorta conflict with the deduction of keys (you would use the most "low" common key for all those same translations)
      • makes the CZ problem perhaps more difficult, since the translator then can't rely on keys being defined in the english file
  • Is basename actually needed ? Keys are long and unique
    • Global chains of message bundles - look into all known bundles
    • Basename helps grouping translation work - "I am now translating module X" - but that doesn't mean the basename has to be specified necessarily
    • Order of message bundles chain would need to be consistent and predictable
  • Enable inline translations within a dialog
  • Have a Magnolia-hosted tool to replace the google spreadsheet
    • some rules like "a translation needs to be validated by 2 other persons to be applied", "once applied it can't be changed directly - only via a 'request'", ...
    • could have a "MessagesManager" impl that fetches translations from this service

Suggestions

Differentiate between in-code-translations, vs in-config-translations. Translations in-configuration will require very little code change, but will require some work on update tasks and bootstrap files. IntelliJ (Eclipse too, probably) provides analysis tools to find out where i18n strings have been hardcoded. Attached a sample report executed on a couple of modules: i18-analysis.zip (main, ui, activation, cache, imaging, workflow). Note that the html-exported report is not very useable, but the in-app report is much more practical: Screen Shot 2013-07-31 at 14.21.14.png

In-code translations

I'm not sure we have a lot of those at the moment, but for things that don't need to be configured, we could. We could benefit from a tool like Localizer or GWT's i18n generator tool. These tools generate code (interfaces and impl) based on the keys found in a message bundle file. They are well thought out, in that they provide the correct methods depending on parameters found in the keys, for example. (generate a String getFileCount(int count) based on a file.count=There are {0} files message, for example). This is great for code completion and type-safety. However, their generated code isn't ioc-friendly, and tends to be laced with static dependencies. These tools also don't handle changes to the generated code well. Which means that if you need to add a message - or change its name, or change its "signature" - you need to edit the properties file, rather than the code.

If we have a lot of these, we could think about rolling out our own tool - or write code that behaves similarly.

In-code translations also include things like the login form, where the i18n call might currently be done in the FreeMarker template.

The login form also has this special construct for translating LoginException messages - to be taken into account.

In-template translations

Currently, some template components which need a translated text item which is not meant to be translated by authors; "Skip" or "Read more" type of labels. Those currently suffer from the fact they are in the same message bundle (i18nBasename) as their respective template definition; as such, if one wants to change those text items for a given project, they have to either copy the complete STK message bundles, or customize the components. Neither is ideal. Currently, at least for FreeMarker templates, we pass definition.getI18nBasename() to info.magnolia.freemarker.FreemarkerHelper#addDefaultData, which adds an i18n object to the FM context with the definition's message bundle.

It should be possible for templates to use a different message bundle than the one used to translate the definitions' labels and descriptions.

In-config translations

A complete code review is required to identify all places with a direct String output (such as button labels, column headers, etc.). Such places have to be replaced with a proper i18n mechanism (MessagesManager.getWithDefault(key, defaultMsg), where the original String will be used as defaultMsg).

A complete manual code review is not necessary*. A search for usages of the following (to be completed) should cover 95% of our bases.

  • info.magnolia.ui.form.field.definition.FieldDefinition#getLabel
  • info.magnolia.ui.form.field.definition.FieldDefinition#getDescription
  • info.magnolia.ui.form.definition.TabDefinition#getLabel
  • ...

*there is an evident need for a complete review of all places impact by the changes proposed in this concept, but the goal would not be to discover where we need to apply i18n. A review could be done before or after, simply with the goal of simplifying, or making more consistent, the existing code base. 

Convention over configuration

We want to introduce a naming convention, both for basenames (i.e the location of the message files) and the key themselves.

To avoid a lot of redundant and verbose configuration, we could take this one step further and use the conventional name - if none is configured - to lookup a particular text. This will also help for cases where the dialog configuration is (very!) redundant (99% of dialog actions are "save" and "cancel" - these are currently configured for each and every dialog, making maintenance a nightmare)

With some clever fallback mechanisms, this could make translations easier and simpler.

Suggestions for a fallback chain are given below. Given a bundle, the keys are "tried" in order. The first value to be found is used.

If fields or other elements need to be empty, like today, the label has to be configured as "empty", or the corresponding key.

The "generated" key should reflect the real "location" of an element, not where it's inherited from. e.g the "save" action label of dialog foobar should first be looked up at <dialog-name>.actions.save before actions.save. Chains for inherited elements should however (ideally) also lookup the parent's key. 

If performance becomes a problem (with all the fallbacks and the chained message bundles), we could introduce a simple caching mechanism.
As a counter example, here's what we've been doing so far, taken from STK: dialogs.pages.faq.stkFAQHeader.tabMain.title.label:
  • The dialogs prefix is not relevant and noisy. It was historically introduced to separate those labels from the page templates and page components names and description. Indeed, we're likely to have a stkFAQHeader.name somewhere. Currently leaning towards using separate message bundles. Or have an non-mandatory prefix/suffix (i.e there's a chance the component's title/name needs the same label as its first tab ?)
  • The pages.faq statement is arbitrary and is derived from the fact that the dialog in question happens to be configured under an arbitrary folder structure ( pages/faq/ )

Date formats and other localized items

We should make sure things like ColumnFormatter not only use the current user's locale, but also that this is indeed an "enabled" locale. A ui entirely in english but with a date formatted in french would be silly.

i18nBasename or message bundles

i18nBasename is the property we use to look up a message bundle. This tells the system where the translation files are. It is called "basename" because i18n mechanisms typically append the locale to this and use the result as a path to a file (eg lookup <mybasename>_de_CH.properties, then <mybasename>_de.properties, then <mybasename>.properties - some even go as far as going up the path hierarchy (parent folders) of the basename until they find what they're looking for)

Up to Magnolia 4.5, the i18nBasename property was defined in a dialog definition (or further down). With 5.0, this exists in DialogDefinition, but also in one level below (in FormDefinition), and still exists in all elements below (TabDefinitionFieldDefinition, ...). The property is also present in ActionDefinition (members of DialogDefinition).

In 99% of the cases, the property set at dialog level should be enough. It is useful to keep the possibility to redefine it in actions, forms, tabs, and fields, but it should not be necessary.

We could take this further and create a naming convention for i18nBasename as well (see below)

However, actions definitions also need to be i18n'd. For apps, it would make sense to have i18nBasename defined at the app level (AppDescriptor). Dialogs however are not necessarily tied to an app (question) i.e they're used by the page-editor, so we wouldn't want to use the page-editor's i18nBasename.

Defining i18nBasename at module level would be ideal - in terms of minimalizing redundancy anyway - but I'm not sure we'd have support for that right now. It'd be interesting to have i18nBasename in a module descriptor though. It would still be possible for individual components to override it if needed.

getI18nBasename is defined in too many places. It's inconsistent and unintuitive. Why the redundancy between info.magnolia.ui.dialog.Dialog#getI18nBasename and info.magnolia.ui.dialog.definition.DialogDefinition#getI18nBasename for example ?

getLabel() vs getI18nBasename() vs usages (question)

Each translatable item currently has a getLabel (or getWhatever) method, some have several (getDescription). They also have a getI18nBasename() method. This combination makes it unclear whose responsibility it is to actually translate (i.e call MessagesManager) the piece of text. Should it the the FieldDefinition itself ? The FieldFactory ? The FormBuilder ? Something we hide in Vaadin (we could wrap com.vaadin.ui.Component#setCaption)

Would it help if we actually removed that stuff from our APIs and use an annotation to indicate which configured Strings need to be translated ? (e.g add an annotation on info.magnolia.ui.form.field.definition.FieldDefinition#getLabelthe title, t

Bootstrap files update

After the code review, the configuration (bootstrap) files have to be reviewed, and values of properties label and description (and alternatively others) will be replaced with a proper message key. Such message key (and value) will be added to the messages_en.properties file of the admincentral UI module (or the other module's messages file), and the path of the property + the key will be added to the list of changed properties (one list for each module, a properties file named i18n_properties_to_update.properties on classpath), which will be then used in the VersionHandlerto replace the values on the upgrade (using new UpdateI18nPropertiesTask).

Based on existing (or then existing) translation files, we should be able to write a script that does a search/replace into bootstrap files. And/or a (temporary) update task could help regenerating them.

Migration

Based on existing (or then existing) translation files, it should be easy to update content. (find i18nBasename, find values for "label" and "description" properties, reverse-lookup, replace by corresponding key)

MessagesManager implementation

info.magnolia.cms.i18n.MessagesManager is the component that has been used to handled i18n up until Magnolia 4.5. It has its flaws, and could be rewritten.

  • it tries to cover too many use-cases (either enforce passing a Locale, or never pass one)
  • its responsibilities are not clear - it observes and holds some i18n config (but not all?) and at the same time provides translation support
  • DefaultMessagesManager is still very tied to using property files.
  • DefaultMessagesManager isn't cleanly decoupled from the system - it's still using content2bean "manually", etc; it's not a real "component".
  • rething package name and/or class name (package mixes i18n for UI and for content, to start with...)
  • It's not easy to test
  • Some logic is burried in the MessagesChain class - which is where, for instance, we've been appending the ??? when a key wasn't found so far. Except that we've seen workarounds popup here and there to remove those, etc..
  • info.magnolia.cms.i18n.MessagesUtil proposes too many methods; as a result, we're completely inconsistently use those in many places in our code. Get rid of this.
  • http://jira.magnolia-cms.com/issues/?jql=text%20~%20%22messagesmanager%22%20and%20resolution%3DUnresolved

Suggested key patterns

The below is a rough outline. The 3 goals below are somewhat hard to reach all at once.

  • avoid redundancy or noise (no dialog. prefix, ...)
  • avoid conflicts (but allow them - on purpose - for labels that are actually meant to be the same in most situations)
  • be consistent (this part is hard - sometimes we prefix with module name, sometimes with app, sometimes with nothing)

Field labels: – optional fallback to a key without a .label suffix to make things less verbose

<dialog-name>.<tab-name>.<field-name>.label
<dialog-name>.<tab-name>.<field-name>
<dialog-name>.<field-name>.label
<dialog-name>.<field-name>

Field descriptions - here we can't fallback to a key without the .desc suffix

<dialog-name>.<tab-name>.<field-name>.desc
<dialog-name>.<field-name>.desc

Tab labels:

<dialog-name>.<tab-name>.label (or .tablabel for explicitness?)

Action labels:

(warning) 99% of our dialogs have the same save/cancel actions. Those should be defaults. Labels should still be overridable on a dialog-per-dialog basis.

<dialog-name>.actions.<action-name>.label
<dialog-name>.actions.<action-name>

(lightbulb) I introduced the .actions portion here to avoid confusion with fields; consistency would dictate having a .fields or .tabs portion for field and tabs labels too, but that would downplay the conciseness.

(lightbulb) for all dialog-related items, we could also use <dialog-id> and fallback to dialog-name, for further specializing. (warning) Dialog ID is possibly currently not available; if it is, it's a single string concatenating module name and dialog name, which isn't ideal. It'd be sweet to be able to get back to module id (and app id) from a dialog.

Apps:

<app-id>.label
<app-id>.icon # because the icon might have to be localized
<app-id>.description # because a mouse-over title of the app might be interesting ?

<app-id> could be <module-id>.<app-name> or just app-name (same as for dialogs)

App launcher groups:

app-launcher.<app-group-name>.label # we use the app-launcher prefix, as if app-launcher was an app (which we should consider considering, I suppose)

Templates:

<module-name>.<template-name>.title # I think "title" is what we've been using in 4.x - we could use label for consistency, or simply name
<module-name>.<template-name> # same as above
<module-name>.<template-name>.desc # Useful in template selector

(lightbulb) <module-name>.<template-name> is essentially the component ID.

Page components:

<module-name>.<component-name>.title # see remark above
<module-name>.<component-name>
<module-name>.<component-name>.desc

(lightbulb) <module-name>.<component-name> is essentially the component ID.

Workbench columns:

<app-id>.<sub-app-name>.views.<view-name>.<column-name>
Example: configuration.browser.views.list.name for /modules/ui-admincentral/apps/configuration/subApps/browser/workbench/contentViews/list/columns/name/label

Other elements tbd ?
  • actions in dialog
  • actionbar in subapps
  • workbench/<view>/columns in content apps: column names
  • in workbench/view/columns, we also have formatterClass which should be locale-sensitive
  • app (label in app launcher, tab)
  • page templates
  • page components

Suggested i18nBasename patterns

  • As defined in module descriptor
  • /mgnl-i18n/<module-name>/messages
  • No labels