[rfc][icedtea-web] Replace single quote with two single quotes when using MessageFormat in Translator

[Jiri Vanek]

On 03/17/2015 04:34 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> On 03/16/2015 09:31 PM, Jie Kang wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> On 03/16/2015 07:20 PM CET Jie Kang wrote:
>>>>> ----- Original Message -----
>>>>>> On 03/16/2015 05:32 PM CET Jie Kang wrote:
>>>>>>> ----- Original Message -----
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> When looking into reproducer failures I noticed that the single quote
>>>>>>>> '
>>>>>>>> was
>>>>>>>> not being displayed in the logs.
>>>>>>>>
>>>>>>>> E.g:
>>>>>>>>
>>>>>>>> don't --> dont
>>>>>>>>
>>>>>>>> This was caused by the use of MessageFormat in the Translator class.
>>>>>>>>
>>>>>>>>     From [1]: Within a String, "''" represents a single quote. [...]
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> http://docs.oracle.com/javase/6/docs/api/java/text/MessageFormat.html
>>>>>>>>
>>>>>>>> Out of the various possible fixes, I have decided to replace single
>>>>>>>> quote
>>>>>>>> instances with two quotes only prior to using the MessageFormat class.
>>>>>>>> Afaict, this doesn't cause any issues and doesn't affect things that
>>>>>>>> don't
>>>>>>>> go through the MessageFormat class.
>>>>>>>>
>>>>>>>> This bug caused a number of reproducers to fail (ex.
>>>>>>>> CodeBaseManifestEntryUnsignedNotMatching.BrowserJNLPHrefRemoteTest) as
>>>>>>>> they
>>>>>>>> expected a string like "don't" but the log output contained a string
>>>>>>>> "dont".
>>>>>>>> With this fix, those reproducers now pass.
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>
>>>>>> Hmm, the solution to this problem actually depends on the coding style
>>>>>> of
>>>>>> message property files. For example, OpenJDK works around this issue is
>>>>>> by
>>>>>> actually putting double apostrophe characters into their tool's message
>>>>>> property
>>>>>> files. So, IcedTea-Web should have or decide on some sort of coding
>>>>>> style
>>>>>> for
>>>>>> its message property files. Which one should it be is arguable. Your
>>>>>> approach
>>>>>> is
>>>>>> certainly valid, however whether it also acceptable depends on the
>>>>>> coding
>>>>>> style
>>>>>> for message property files in IcedTea-Web. Personally, I would prefer
>>>>>> OpenJDK's
>>>>>> approach, hence modify the property files and do not get into possibly
>>>>>> error
>>>>>> prone mangling of strings.
>>>>>
>>>>> I prefer users of the property files not having to deal with how the code
>>>>> formats the strings. I don't care which method goes in as long as one
>>>>> goes
>>>>> in so I'll wait for a third opinion.
>>>>
>>>> I understand and I would like this to happen too. However, you would also
>>>> have
>>>> to provide for proper handling of explicit occurrences of curly brackets
>>>> (U+007B
>>>> and U+007D). I am sure we do not want to get into this game.
>>>
>>> Sure. Patch attached. Comments have been added to Messages.properties. I
>>> chose to keep the test for apostrophe escaping and modified it
>>> appropriately. That way if someone modifies this later it'll fail and
>>> hopefully point them towards fixing the properties files as well.
>>>
>>> I used:
>>>
>>>    sed -i "s/'/''/g" $file
>>>
>>> to change all the properties files.
>>>
>>
>> Yes - all those messages should be  verfied manually in runtime. . Also in
>> manpages/html docs and
>> palintext docs. And tehirs representation in intergrated help....
>>
>
> Hello,
>
>
> I manually verified all the changed properties to continue to work.
>
> Patch attached again. Thoughts?
>
>
> Also with this patch on HEAD:
>
> Passed: CodeBaseManifestEntrySignedMatching.ApplicationJNLPLocalTest
> Passed: CodeBaseManifestEntrySignedMatching.AppletJNLPRLocalTest
> Passed: CodeBaseManifestEntrySignedMatching.ApplicationJNLPRemoteTest
> Passed: CodeBaseManifestEntrySignedMatching.BrowserAppletLocalTest - firefox
> Passed: CodeBaseManifestEntrySignedMatching.BrowserJNLPHrefRemoteTest - firefox
> Passed: CodeBaseManifestEntrySignedMatching.BrowserJNLPHrefLocalTest - firefox
> Passed: CodeBaseManifestEntrySignedMatching.AppletJNLPRemoteTest
> Passed: CodeBaseManifestEntrySignedMatching.BrowserAppletRemoteTest - firefox
> Passed: CodeBaseManifestEntrySignedMatching.ApplicationJNLPLocalTestWithRemoteCodebase
> Passed: CodeBaseManifestEntryUnsignedNotMatching.ApplicationJNLPLocalTest
> Passed: CodeBaseManifestEntryUnsignedNotMatching.AppletJNLPRLocalTest
> Passed: CodeBaseManifestEntryUnsignedNotMatching.ApplicationJNLPRemoteTest
> Passed: CodeBaseManifestEntryUnsignedNotMatching.BrowserAppletLocalTest - firefox
> Passed: CodeBaseManifestEntryUnsignedNotMatching.BrowserJNLPHrefRemoteTest - firefox
> Passed: CodeBaseManifestEntryUnsignedNotMatching.BrowserJNLPHrefLocalTest - firefox
> Passed: CodeBaseManifestEntryUnsignedNotMatching.AppletJNLPRemoteTest
> Passed: CodeBaseManifestEntryUnsignedNotMatching.BrowserAppletRemoteTest - firefox
> Passed: CodeBaseManifestEntryUnsignedNotMatching.ApplicationJNLPLocalTestWithRemoteCodebase
> Passed: CodeBaseManifestEntrySignedNotMatching.ApplicationJNLPLocalTest
> Passed: CodeBaseManifestEntrySignedNotMatching.AppletJNLPRLocalTest
> Passed: CodeBaseManifestEntrySignedNotMatching.ApplicationJNLPRemoteTest
> Passed: CodeBaseManifestEntrySignedNotMatching.BrowserAppletLocalTest - firefox
> Passed: CodeBaseManifestEntrySignedNotMatching.BrowserJNLPHrefRemoteTest - firefox
> Passed: CodeBaseManifestEntrySignedNotMatching.BrowserJNLPHrefLocalTest - firefox
> Passed: CodeBaseManifestEntrySignedNotMatching.AppletJNLPRemoteTest
> Passed: CodeBaseManifestEntrySignedNotMatching.BrowserAppletRemoteTest - firefox
> Passed: CodeBaseManifestEntrySignedNotMatching.ApplicationJNLPLocalTestWithRemoteCodebase
> Passed: CodeBaseManifestEntryUnsignedMatching.ApplicationJNLPLocalTest
> Passed: CodeBaseManifestEntryUnsignedMatching.AppletJNLPRLocalTest
> Passed: CodeBaseManifestEntryUnsignedMatching.ApplicationJNLPRemoteTest
> Passed: CodeBaseManifestEntryUnsignedMatching.BrowserAppletLocalTest - firefox
> Passed: CodeBaseManifestEntryUnsignedMatching.BrowserJNLPHrefRemoteTest - firefox
> Passed: CodeBaseManifestEntryUnsignedMatching.BrowserJNLPHrefLocalTest - firefox
> Passed: CodeBaseManifestEntryUnsignedMatching.AppletJNLPRemoteTest
> Passed: CodeBaseManifestEntryUnsignedMatching.BrowserAppletRemoteTest - firefox
> Passed: CodeBaseManifestEntryUnsignedMatching.ApplicationJNLPLocalTestWithRemoteCodebase
> Total tests run: 36; From  those : 0 known to fail
> Test known to fail: passed: 0; failed: 0; ignored: 0
> Test results: passed: 36; failed: 0; ignored: 0
>
> Yay :D
>
> Hopefully it's not just my machine.

Wou!


Please push asap. I will request an testmachine and lunchit. Hopefully results will be tomorow 
before meeting!

Thanx!

J.
>
>
> Regards,
>
>> J.
>>
>>
>