> -----Original Message----- > From: huizhe wang [mailto:huizhe.w...@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.fu...@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.fu...@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.w...@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.w...@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 > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>> > >>> > >
