RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303

Frank Yuan frank.yuan at oracle.com
Wed Feb 15 00:33:15 UTC 2017


> -----Original Message-----
> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> Subject: Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> 
> Looks good to me as well.
> 
> For the CR and LF question, XML processors are required by the
> specification to normalize and translate both CRLF sequence and any CR
> not followed by LF to a single LF.  For that reason, you don't have
> check CR since the content the serializer gets is parsed.
> 
Thank you very much for explanation, since that, I will remove the check for CR as below:
3439c3439
<                     while (skipBeginningNewlines && (text[start] == '\n' || text[start] == '\r')) {
---
>                     while (skipBeginningNewlines && text[start] == '\n') {
3498c3498
<                         while (skipBeginningNewlines && (text[start] == '\n' || text[start] == '\r')) {
---
>                         while (skipBeginningNewlines && text[start] == '\n') {

Once it is passed by all tests, I will push the change.

Thanks
Frank

> Best,
> Joe
> 
> On 2/14/2017 6:58 AM, Frank Yuan wrote:
> >> -----Original Message-----
> >> From: Daniel Fuchs [mailto:daniel.fuchs at oracle.com]
> >> Subject: Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> >>
> >> Hi Frank,
> >>
> >> On 14/02/17 13:43, Frank Yuan wrote:
> >>>> -----Original Message-----
> >>>> From: Daniel Fuchs [mailto:daniel.fuchs at oracle.com]
> >>>> Subject: Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> >>>>
> >>>> Hi Frank,
> >>>>
> >>>> Should you skip '\r' if it's not followed by '\n'?
> >> Well - I'll let Joe answer that. ;-)
> > Hmm, wait for Joe to confirm.
> >
> >> It was just a question, I was wondering whether that could
> >> potentially cause problems down the road - since new lines
> >> are usually only either '\n' or '\r'+'\n'.
> >>
> > Agree.
> >
> >> Your patch looks fine otherwise, maybe the code that skips
> >> the '\n' could be factorized somewhere to avoid duplication,
> >> but that's not really important.
> >>
> >> Both issues reported in the bug are still fix - so I think we
> >> should try to get this patch in as soon as we can.
> >>
> > Yes, understand!
> >
> > Thanks
> > Frank
> >
> >> best regards,
> >>
> >> -- daniel
> >>
> >>> Does it matter? Since XML processor should normalize the newline.
> >>>
> >>> Thanks
> >>> Frank
> >>>> best regards,
> >>>>
> >>>> -- daniel
> >>>>
> >>>> On 14/02/17 10:33, Frank Yuan wrote:
> >>>>> Hi Joe
> >>>>>
> >>>>> As you suggested, I made pretty-print a little better based on the fix. That is when adding indentation, just check the
> >>> beginning
> >>>>> character(s), in case of '\n' or '\r' then, ignore it/them.
> >>>>>
> >>>>> Please check the new webrev: http://cr.openjdk.java.net/~fyuan/8174025/webrev.01/
> >>>>>
> >>>>>
> >>>>> Thanks
> >>>>> Frank
> >>>>>
> >>>>> -----Original Message-----
> >>>>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> >>>>> Subject: should have been 8174025 -> Re: RFR [JAXP] JDK-8170192 Regression in XML Transform caused by JDK-8087303
> >>>>>
> >>>>> Note that the bug id was incorrect, it should have been 8174025. 8170192
> >>>>> was a test bug fix.
> >>>>>
> >>>>> -Joe
> >>>>>
> >>>>> On 2/13/2017 1:35 AM, Frank Yuan wrote:
> >>>>>> Hi Joe and Daniel
> >>>>>>
> >>>>>> Thank you very much for your review!
> >>>>>>
> >>>>>> Frank
> >>>>>>
> >>>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> >>>>>> Subject: Re: RFR [JAXP] JDK-8170192 Regression in XML Transform caused by JDK-8087303
> >>>>>>
> >>>>>> +1 from me too.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Joe
> >>>>>>
> >>>>>> On 2/10/2017 5:25 AM, Daniel Fuchs wrote:
> >>>>>>> Hi Frank,
> >>>>>>>
> >>>>>>> Thanks for fixing this!
> >>>>>>>
> >>>>>>> I imported your patch and played with it a bit.
> >>>>>>> Also ran the jaxp test.
> >>>>>>>
> >>>>>>> Both issues reported have indeed disappeared.
> >>>>>>>
> >>>>>>> So that's a +1 from me.
> >>>>>>>
> >>>>>>> best regards,
> >>>>>>>
> >>>>>>> -- daniel
> >>>>>>>
> >>>>>>> On 10/02/17 11:03, Frank Yuan wrote:
> >>>>>>>> Hi All
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Would you like to review
> >>>>>>>> http://cr.openjdk.java.net/~fyuan/8174025/webrev.00/?
> >>>>>>>>
> >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8174025
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> JDK-8087303 introduced 2 issues:
> >>>>>>>>
> >>>>>>>> 1.       Flaw when xlst uses disable-output-escaping attribute
> >>>>>>>>
> >>>>>>>> 2.       Eat the whitespace between html inline elements
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> This patch fixed the issues.
> >>>>>>>>
> >>>>>>>> To fix the second issue, we decide to keep the compatibility with JDK 8
> >>>>>>>> on the whitespace handling, that is only LSSerializer cleans the extra
> >>>>>>>> whitespaces in if pretty-print is on, but XSLT doesn't.
> >>>>>>>>
> >>>>>>>> I modified the behavior of getIndent() method in class ToStream, to make
> >>>>>>>> LSSerializer be sensitive of current state from ToStream. This should be
> >>>>>>>> safe because ToStream is an internal class and getIndent() method is
> >>>>>>>> never used before.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks
> >>>>>>>>
> >>>>>>>> Frank
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>
> >>>
> >




More information about the core-libs-dev mailing list