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

Tue Mar 17 16:13:16 UTC 2015

On 03/17/2015 05:06 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> 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!
>
> Ah I realized there is still the issue with manifest-checking.
>
> The tests expect manifest checking enabled, but I think the reproducers usually run with them disabled so there will probably be failures on the test machine :\
>
> I think ldracz is working on something to fix this though (discussion in thread [rfc][icedtea-web] Add comments about permission attributes not being checked in reproducers).
>
oh yes:(

still - the approach here is to
@beforeclass
  - ensure they are enabled
@afterclass - restore original file.

You can do this now.  But later you need to adapt to lukas' changeset

thanx!

J.