RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
Thank you very much for the check! Pushed.
Frank
> -Original Message-
> From: huizhe wang [mailto:huizhe.w...@oracle.com]
> Sent: Wednesday, February 15, 2017 11:35 AM
> To: Frank Yuan; 'Daniel Fuchs'
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by
> JDK-8087303
>
> Sounds good. The change to check LF only looks good.
>
> Thanks,
> Joe
>
> On 2/14/2017 7:22 PM, Frank Yuan wrote:
> > Hi Joe and Daniel
> >
> > I tested more and checked the code, both of you are right.
> >
> > CRLF, CR, LF in the xml file are all normalized to LF, and finally output
> > to the platform-related line.separator if it's
serialized.
> > The serializer only handle LF as newline. User can create CR text node
> > programmatically or there may be some case that CR is
sent to
> > the serializer intentionally, but anyway in these cases, CR is not regarded
> > as a newline. So the change as Joe's comment and I
> > stated in previous mail is made:
> > http://cr.openjdk.java.net/~fyuan/8174025/webrev.02/
> >
> > Thanks
> > Frank
> >
> >> -----Original Message-
> >> From: Frank Yuan [mailto:frank.y...@oracle.com]
> >> Subject: RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by
> >> JDK-8087303
> >>
> >>
> >>> -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
> >>
Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
Sounds good. The change to check LF only looks good.
Thanks,
Joe
On 2/14/2017 7:22 PM, Frank Yuan wrote:
Hi Joe and Daniel
I tested more and checked the code, both of you are right.
CRLF, CR, LF in the xml file are all normalized to LF, and finally output to
the platform-related line.separator if it's serialized.
The serializer only handle LF as newline. User can create CR text node
programmatically or there may be some case that CR is sent to
the serializer intentionally, but anyway in these cases, CR is not regarded as
a newline. So the change as Joe's comment and I
stated in previous mail is made:
http://cr.openjdk.java.net/~fyuan/8174025/webrev.02/
Thanks
Frank
-Original Message-
From: Frank Yuan [mailto:frank.y...@oracle.com]
Subject: RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by
JDK-8087303
-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
RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
Hi Joe and Daniel
I tested more and checked the code, both of you are right.
CRLF, CR, LF in the xml file are all normalized to LF, and finally output to
the platform-related line.separator if it's serialized.
The serializer only handle LF as newline. User can create CR text node
programmatically or there may be some case that CR is sent to
the serializer intentionally, but anyway in these cases, CR is not regarded as
a newline. So the change as Joe's comment and I
stated in previous mail is made:
http://cr.openjdk.java.net/~fyuan/8174025/webrev.02/
Thanks
Frank
> -Original Message-
> From: Frank Yuan [mailto:frank.y...@oracle.com]
> Subject: RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by
> JDK-8087303
>
>
> > -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/
> > >>>>>
> > >>>&
RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> -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
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. 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
RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> -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 > >>>>>> > >>>>>> > >>>>>> > >>>> > >>> > >>> > > > >
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. ;-) 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'. 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. 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
RE: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
> -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'? > 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 > >>>> > >>>> > >>>> > >> > > > >
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'? 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
Re: RFR [JAXP] JDK-8174025 Regression in XML Transform caused by JDK-8087303
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 >>> >>> >>> >
