[LANG-42] Correct i18n messages to be valid for formatting using MessageFormat Created: 13/Jan/16  Updated: 06/Feb/17  Resolved: 23/Dec/16

Status: Closed
Project: Language Bundles
Component/s: None
Affects Version/s: None
Fix Version/s: 1.0.9

Type: Task Priority: Neutral
Reporter: AntonĂ­n Juran Assignee: Hieu Nguyen Duc
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 9d 3.25h
Original Estimate: 3d

Issue Links:
Cloners
is cloned by MGNLADVCACHE-82 CLONE - Correct i18n messages to be v... Closed
is cloned by MGNLCACHE-159 CLONE - Correct i18n messages to be v... Closed
is cloned by MGNLSLOCK-43 CLONE - Correct i18n messages to be v... Closed
Relates
relates to LANG-63 Page delete confirmation message in d... Closed
relation
is related to MGNLUI-3701 IllegalArgumentException is thrown wh... Closed
is related to MAGNOLIA-6489 IllegalArgumentException is thrown wh... Closed
is related to MAGNOLIA-8022 Use MessageFormatterUtils instead Mes... Open
supersession
supersedes LANG-64 Resource delete confirmation message ... Closed
supersedes MGNLUI-4109 Incorrectly displayed message during ... Closed
Template:
Acceptance criteria:
Empty
Task DoR:
Empty
Date of First Response:
Sprint: Saigon 75
Story Points: 5

 Description   

namely:

  • doubling single-quotes intended as literals
  • appending a space character after angle-brackets used in MessageFormat pluralization (to work around our own defensive HTML sanitation)
  • double-check for correct "choice syntax", see LANG-64 for an eager translation problem


 Comments   
Comment by Hieu Nguyen Duc [ 09/Dec/16 ]

1) Reason why we have to append a space after angle brackets:

Because DefaultMessageBundlesLoader cleans malicious HTML using JSoup#clean, we have to append a space after angle brackets to make sure the message is not recognized as HTML tag.
=> Add spaces for all messages having "choice" syntax.

Ref: MAGNOLIA-6728

2) Regarding lacking of space, we have a special case that doesn't need a space.

The content is wrapped by single quotes.

order=Please buy {0} {0,choice,1#'glass'|1<'glasses'} of beer for {1,choice,19#dinner|13>'lunch'}.

But adding a space doesn't cause any problem.
=> Add space also for this case.

3) Because lacking of space doesn't show any error

We have to test it visually.
=> I created a tab in Showcase-app to check it in Magnolia environment.
https://git.magnolia-cms.com/projects/INTERNAL/repos/showcase-app/pull-requests/1/overview

4) DefaultMessageBundleLoader users should be aware of this issue.

I've added some test cases for the issue lacking of space.
https://git.magnolia-cms.com/projects/PLATFORM/repos/main/pull-requests/349/overview

Comment by Ngoc Nguyenthanh [ 12/Dec/16 ]

Groovy scripts support for testing and finding potential error keys & messages

1. List down potential wrong format messages
  • Load all message files and try to apply with dummy parameters
  • Prefix "Error: ". We need to check by manually one by one on this case always.
  • With the prefix "Potential: " syntax error: maybe it's issue with single quote or something else
  • Know issue: Have no clue why it can't load all locales (1/2 maybe)
import info.magnolia.objectfactory.Components;
import info.magnolia.i18nsystem.DefaultMessageBundlesLoader;
import info.magnolia.resourceloader.ResourceOrigin;
import java.util.Locale;
import java.util.Properties;
import java.text.MessageFormat;

componentProvider = Components.getComponentProvider();

messageBundles = componentProvider.newInstance(DefaultMessageBundlesLoader.class, Components.getComponent(ResourceOrigin.class));
messages = messageBundles.getMessages();
for (Map.Entry<Locale, Properties> entry : messages.entrySet())
{
     locale = entry.getKey();
     println "Language: " + locale;
     localeMessages = entry.getValue();
     for (Object name : Collections.list(localeMessages.propertyNames())) {
         String key = name.toString();
         String message = localeMessages.get(name);
         //println key + " = " + message;
         try {
            def args = [0,1,2,3,4] as Object[];
            formatted = new MessageFormat(message, locale).format(args);
           if(formatted.contains("{")){
               println "Potential: " + key + " = " + message;
           }

        } catch (IllegalArgumentException e) {
            println "Error: " + key + " = " + message;
        }
     }

     println "";
}
2. Quick test a message with MessageFormat
  • Need more accurate, modify the script mentioned above to get real message in Magnolia
  • Just for quick check purpose
        import org.jsoup.Jsoup;
        import org.jsoup.safety.Whitelist;
        import java.text.MessageFormat;
        String message = "Please buy {0} {0,choice,1#'glass'|1< 'glasses'} of beer for {1,choice,19#dinner|13>'lunch'}.";
        if (!Jsoup.isValid(message, Whitelist.basic())){
         message = Jsoup.clean(message, Whitelist.basic());
        }
        try {
            args = [0,1, 2,3,4] as Object[];
            formatted = new MessageFormat(message).format(args);
        } catch (Exception e) {
            println "Error: ";
            println e;
        }
Comment by Ngoc Nguyenthanh [ 16/Dec/16 ]

Scan in running instance may not accurate due to missing modules. Scan a source code folders for

import info.magnolia.cms.i18n.MessagesManager
import info.magnolia.i18nsystem.util.LocaleUtils

import java.text.MessageFormat

import static groovy.io.FileType.FILES

folders = ["/Stephen/Magnolia/MagnoliaSourceCode/Lang/lang",
           "/Stephen/Magnolia/MagnoliaSourceCode/Modules-Dev",
           "/Stephen/Magnolia/MagnoliaSourceCode/Blossom",
           "/Stephen/Magnolia/MagnoliaSourceCode/Enterprise-Dev",
           "/Stephen/Magnolia/MagnoliaSourceCode/Platform-5.5"
           ];
folders.each {
    folder ->
        new File(folder).eachFileRecurse(FILES) {
            if (it.name.endsWith('.properties')) {
                locale = LocaleUtils.parseFromFilename(it.name, MessagesManager.getInstance().getDefaultLocale());
                // println "Language: " + it;
                it.eachLine {
                    message ->
                        try {
                            def args = [0, 1, 2, 3, 4] as Object[];
                            formatted = new MessageFormat(message, locale).format(args);
                            if (formatted.contains("{")) {
                                println "Potential: " + message;
                            }

                        } catch (IllegalArgumentException e) {
                            println "Error: " + message;
                        }

                }
            }
        }
}
Generated at Mon Feb 12 02:19:10 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.