Wicket
  1. Wicket
  2. WICKET-1319

StringResourceModel incorrectly escapes ' characters in choice formats

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0-final
    • Fix Version/s: 1.3.2
    • Component/s: wicket
    • Labels:
      None

      Description

      Here is a sample format that breaks when migrating from 1.2.6 to 1.3. We've currently worked around it by locating the string and using MessageFormat ourselves:

      foo.label={0,choice,-1#n/a|-1<'

      {1} {0,number,#,##0.00}

      '}

      1. patch.txt
        2 kB
        Johan Compagner

        Activity

        Hide
        Johan Compagner added a comment -

        i guess you use it like this:

        StringResourceModel("label", this, null, new Object[]

        { new Integer(10) }

        ))

        so by giving the param that uses the MessageFormat class. We also seem to have PropertySubstitution ourslfs by giving a model with it
        its a bit confusing to have 2 things that sort of do the same thing...

        But i see gerolf did change the ' escaping it moved it from PropertyVariableInterpolator to StringResourceModel (MessageFormat part) but those are not really related to each other...

        Gerolf can you take a look? (revision 596906: moved single-quote-escaping from PropertyVariableInterpolator to StringResourceModel)

        Show
        Johan Compagner added a comment - i guess you use it like this: StringResourceModel("label", this, null, new Object[] { new Integer(10) } )) so by giving the param that uses the MessageFormat class. We also seem to have PropertySubstitution ourslfs by giving a model with it its a bit confusing to have 2 things that sort of do the same thing... But i see gerolf did change the ' escaping it moved it from PropertyVariableInterpolator to StringResourceModel (MessageFormat part) but those are not really related to each other... Gerolf can you take a look? (revision 596906: moved single-quote-escaping from PropertyVariableInterpolator to StringResourceModel)
        Hide
        Gerolf Seitz added a comment -

        found a problem with the interaction of PropertyVariableInterpolator and StringResourceModel:
        PVI always escapes single quotes (for use with MessageFormat), whereas SRM only uses MessageFormat if the "Object[] parameters" parameter is not null.

        Always using MessageFormat (even when there's no

        {0}, ... parameter in the resource string) would mean, that users would need to properly escape every single quote in every string. don't think that's a good move.

        don't know if making PVI aware that there might be other kind of variables ({0}

        ...) and only let it escape the result string if such a parameter is present, is the right solution.

        any thoughts?

        Show
        Gerolf Seitz added a comment - found a problem with the interaction of PropertyVariableInterpolator and StringResourceModel: PVI always escapes single quotes (for use with MessageFormat), whereas SRM only uses MessageFormat if the "Object[] parameters" parameter is not null. Always using MessageFormat (even when there's no {0}, ... parameter in the resource string) would mean, that users would need to properly escape every single quote in every string. don't think that's a good move. don't know if making PVI aware that there might be other kind of variables ({0} ...) and only let it escape the result string if such a parameter is present, is the right solution. any thoughts?
        Hide
        Gerolf Seitz added a comment -

        for a quick fix, we could add a field "boolean escapeSingleQuotes" to PropertyVariableInterpolator which indicates whether #getValue should escape single quotes before returning the value.
        however, this would make PVI aware that the strings are postprocessed in a certain way (MessageFormat#format).
        so probably something like a EscapeStrategy in VariableInterpolator would fit for the more general case.
        EscapeStrategy would either contain a Map<Character /toEscape/, String/escape sequence/> or just a "char escapeCharacter" and a list of charactersToEscape...

        any thoughts?

        Show
        Gerolf Seitz added a comment - for a quick fix, we could add a field "boolean escapeSingleQuotes" to PropertyVariableInterpolator which indicates whether #getValue should escape single quotes before returning the value. however, this would make PVI aware that the strings are postprocessed in a certain way (MessageFormat#format). so probably something like a EscapeStrategy in VariableInterpolator would fit for the more general case. EscapeStrategy would either contain a Map<Character / toEscape /, String/ escape sequence /> or just a "char escapeCharacter" and a list of charactersToEscape... any thoughts?
        Hide
        Johan Compagner added a comment -

        i have another patch
        Its also a hack.... But all the test passes and we dont really change any interface except make Localizer
        public String substitutePropertyExpressions(final Component component, final String string,
        final IModel model)

        public.

        Show
        Johan Compagner added a comment - i have another patch Its also a hack.... But all the test passes and we dont really change any interface except make Localizer public String substitutePropertyExpressions(final Component component, final String string, final IModel model) public.
        Hide
        Johan Compagner added a comment -

        I applied my patch for now.
        Maybe in 1.4 we should rewrite Localizer.substitutePropertyExpressions to have an extra param IEscapeStrategy

        Show
        Johan Compagner added a comment - I applied my patch for now. Maybe in 1.4 we should rewrite Localizer.substitutePropertyExpressions to have an extra param IEscapeStrategy
        Hide
        Gerolf Seitz added a comment -

        thanks, Johan.

        Show
        Gerolf Seitz added a comment - thanks, Johan.
        Hide
        Meetesh Karia added a comment -

        Thanks for this fix!

        Show
        Meetesh Karia added a comment - Thanks for this fix!

          People

          • Assignee:
            Unassigned
            Reporter:
            Meetesh Karia
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development