Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

-----

-- review all the above

Different translation "scopes"

We need to differentiate between in-code translatable text, in-template translatable text and in-config translatable text.

In-code translations

Some UI elements are not configured. Their texts can be considered "hardcoded". IntelliJ (Eclipse too, probably) provides analysis tools to find out where i18n strings have been hardcoded. Attached is 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

...

Analysis of current situation

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

Each translatable item currently has a getLabel (or getTitle 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

Different translation "scopes"

We need to differentiate between in-code translatable text, in-template translatable text and in-config translatable text.

In-code translations

Some UI elements are not configured. Their texts can be considered "hardcoded". IntelliJ (Eclipse too, probably) provides analysis tools to find out where i18n strings have been hardcoded. Attached is 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

My initial thought was that for these, we could benefit from a tool like Localizer or GWT's i18n generator tool. Unfortunately, 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 fileMy initial thought was that for these, we could benefit from a tool like Localizer or GWT's i18n generator tool. Unfortunately, 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 (tends to rely on static/threadlocal for Locale retrieval), 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.

...

Translations in-configuration will require very little change to existing code, but will require some work on update tasks and bootstrap files.

Status in 5.0

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

...

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.

Proposal

This concept and proposal focuses on in-configuration translations. Hopefully, the concepts can be applied to in-code translations. In-template translations will be taken into account (i.e facilitate the maintenance of the corresponding message files), but changing the mechanism they use might be considered a "next step".

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:

See #status.

Proposal

This concept and proposal focuses on in-configuration translations. Hopefully, the concepts can be applied to in-code translations. In-template translations will be taken into account (i.e facilitate the maintenance of the corresponding message files), but changing the mechanism they use might be considered a "next step".

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. That convention already exists in a informal way; most items in STK for example have a fairly consistent key naming scheme.

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

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 i18nBasename 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. 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. We could also create a naming convention for i18nBasename as well (see below).

However, with the proposed key naming scheme (as well as with the existing informal one), message keys are distinct enough to consider dropping the need for specifying a message bundle. Proposal:
  • We don't need to specifiy i18nBasename anymore for translatable items. (but we can, at the very least to maintain backwards compatibility)
  • Every module will still have their own message bundle file(s); the system will chain and look for messages in all of these
    • We could imagine having a check that would warn, or even fail, when several bundles contains the same key(s).

Date formats and other localized

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

    Implementation

    Proxy .

     

    ------- review all the below

    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)

    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.

    Message bundles for non-english texts

    TBD

    Implementation

    Introducing a couple of concepts. The basic API will be in its own module in magnolia_main; it doesn't need to be in core, and will maybe not even depend on it. It could even be outside of magnolia_main if there is no dependency to core, but we currently lack a good location for such modules (wink)

    API

    • @I18nable annotation. "Internationalizable": marks any object as a candidate for translations. Used on interfaces/classes such as FieldDefinition. Is inherited.
    • @I18nText annotation. Marks a String as to be translated.
    • I18nKeyGenerator<T> interface. Implementations generate translation keys for <T>.

    Implementation "details"

    • A Guice module which intercepts the creation of objects annotated with @I18nable and proxies them
    • Said proxy intercepts method calls annotated with @I18nText and returns translated values

    Update tools and tasks

    Write a tool which

    • Starts up a repo and a specific (or several) component managers
    • Load up a translation file we want to migrate (or all its language counter parts)
    • Imports a bootstrap file we want to migrate
    • This should instantiate a whole bunch of forms etc
    • Go through these one by one

    Mockup

     

    Status

    Main issue: MGNLUI-1826@Jira

    Topics to validate or research

    • To generate keys for a field, we need to navigate to its parent(s): tab name, dialog name, etc. This is currently not part of the API. Two options
      • The Guice module or proxy takes care of adding that to the objects - by navigating their @I18nable members.
      • Add an I18nParent interface to our classes.
      • Other approach: we've been so far focusing on definition objects; OTOH, "live" objects (field as opposed to field definition) know about their parent already.
    • A form can be used in different contexts. It should be translatable according to context.
      • TBD: details, examples
    • Where do we pass the user context (i.e locale)
      • Can we set it at injection time ?
      • Do we add an interface for retrieving the locale-to-use ? (Which UiContext could implement for example)

    Existing uses and inconsistencies to fix for 5.1

    • (error) info.magnolia.ui.form.AbstractFormItem#getMessages - should not be public
      • Is broken: doesn't use the user locale afaict.
    • (error) info.magnolia.ui.form.AbstractFormItem#getMessage - should not be public
    • (error) AbstractFormItem defines a semi-arbitrary message bundles chain (see AbstractFormItem#UI_BASENAMES)
    • (error) 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.
    • (error) 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).
    • (error) 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.
    • (error) 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 ?

    (tick) 

    (error)

    ...

    ------- review all the below

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

    ...