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

[Jie Kang]

----- 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).


Regards,

> 
> Thanx!
> 
> J.
> >
> >
> > Regards,
> >
> >> J.
> >>
> >>
> >
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern