Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-1319

StringResourceModel incorrectly escapes ' characters in choice formats

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
        jcompagner 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
        jcompagner 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
        seitz 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
        seitz 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
        seitz 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
        seitz 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
        jcompagner 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
        jcompagner 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
        jcompagner 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
        jcompagner 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
        seitz Gerolf Seitz added a comment -

        thanks, Johan.

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

        Thanks for this fix!

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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development