RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML
Hi all, Could you please review the 8268457 bug fixes? The problem is that ToHTMLStream applies processing for non-surrogate pairs to the surrogate pair. This fix changes the processing for non-surrogate pairs to the else condition. - Commit messages: - clean up whitespace - 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML Changes: https://git.openjdk.java.net/jdk/pull/4474/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4474&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268457 Stats: 235 lines in 7 files changed: 223 ins; 9 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4474.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4474/head:pull/4474 PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML
On Fri, 11 Jun 2021 12:42:35 GMT, Masanori Yano wrote: > Hi all, > > Could you please review the 8268457 bug fixes? > > The problem is that ToHTMLStream applies processing for non-surrogate pairs > to the surrogate pair. > This fix changes the processing for non-surrogate pairs to the else condition. The fix looks good to me. For ToHTMLStream, please update the copyright year to 2021 (s/2019/2021) and s/@LastModified: Aug 2019/ @LastModified: June 2021. For the test, please move it to test/jaxp/javax/xml/jaxp/unittest/transform and make it a testng test (add @run testng transform.SurrogateTest, and @Test to each test case). For the test itself, you may replace the try-finally block with a try-with-resources statement. For comparing with the gold files, please take a look at the existing test library, specifically test/jaxp/javax/xml/jaxp/libs/jaxp/library/JAXPTestUtilities.java, and do it the testng way, e.g. Assert.assertTrue(compareWithGold(goldFile, outputFile)); - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML
On Fri, 11 Jun 2021 12:42:35 GMT, Masanori Yano wrote: > Hi all, > > Could you please review the 8268457 bug fixes? > > The problem is that ToHTMLStream applies processing for non-surrogate pairs > to the surrogate pair. > This fix changes the processing for non-surrogate pairs to the else condition. src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHTMLStream.java line 1455: > 1453: } > 1454: else > 1455: */ I just wonder the bug was caused by this commenting out, which erroneously removed the `else` here. If the comment-out portion is no longer needed, we could just delete it for good. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML
On Fri, 11 Jun 2021 12:42:35 GMT, Masanori Yano wrote: > Hi all, > > Could you please review the 8268457 bug fixes? > > The problem is that ToHTMLStream applies processing for non-surrogate pairs > to the surrogate pair. > This fix changes the processing for non-surrogate pairs to the else condition. Thank you for reviewing. I reflected the review comments to my change. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v2]
> Hi all, > > Could you please review the 8268457 bug fixes? > > The problem is that ToHTMLStream applies processing for non-surrogate pairs > to the surrogate pair. > This fix changes the processing for non-surrogate pairs to the else condition. Masanori Yano has updated the pull request incrementally with one additional commit since the last revision: Reflect the review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/4474/files - new: https://git.openjdk.java.net/jdk/pull/4474/files/81975b58..d5792a87 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4474&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4474&range=00-01 Stats: 266 lines in 8 files changed: 104 ins; 160 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4474.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4474/head:pull/4474 PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v2]
On Fri, 18 Jun 2021 04:56:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > Reflect the review comments src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHTMLStream.java line 1454: > 1452: writer.write(ch); // no escaping in this case > 1453: } > 1454: else I was suggesting removing the entire comment-out block if it is not needed (and confusing), but I will defer the decision to Joe. test/jaxp/javax/xml/jaxp/unittest/transform/SurrogateTest1.xml line 4: > 2: > 3: 𠮟 > 4: Add a new line at the end of the file (and other newly created files too). - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v2]
On Fri, 18 Jun 2021 20:27:13 GMT, Naoto Sato wrote: >> Masanori Yano has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reflect the review comments > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHTMLStream.java > line 1454: > >> 1452: writer.write(ch); // no escaping in this case >> 1453: } >> 1454: else > > I was suggesting removing the entire comment-out block if it is not needed > (and confusing), but I will defer the decision to Joe. I agree. It's very obsolete. The comment-out block from line 1445 to 1454 can be removed. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
> Hi all, > > Could you please review the 8268457 bug fixes? > > The problem is that ToHTMLStream applies processing for non-surrogate pairs > to the surrogate pair. > This fix changes the processing for non-surrogate pairs to the else condition. Masanori Yano has updated the pull request incrementally with one additional commit since the last revision: remove unnecessally comments and add eof line - Changes: - all: https://git.openjdk.java.net/jdk/pull/4474/files - new: https://git.openjdk.java.net/jdk/pull/4474/files/d5792a87..1183c2f6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4474&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4474&range=01-02 Stats: 14 lines in 4 files changed: 0 ins; 11 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4474.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4474/head:pull/4474 PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Fri, 18 Jun 2021 21:09:39 GMT, Joe Wang wrote: >> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHTMLStream.java >> line 1454: >> >>> 1452: writer.write(ch); // no escaping in this case >>> 1453: } >>> 1454: else >> >> I was suggesting removing the entire comment-out block if it is not needed >> (and confusing), but I will defer the decision to Joe. > > I agree. It's very obsolete. The comment-out block from line 1445 to 1454 can > be removed. I was mistaken. I deleted the entire comment. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v2]
On Fri, 18 Jun 2021 20:28:06 GMT, Naoto Sato wrote: >> Masanori Yano has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reflect the review comments > > test/jaxp/javax/xml/jaxp/unittest/transform/SurrogateTest1.xml line 4: > >> 2: >> 3: 𠮟 >> 4: > > Add a new line at the end of the file (and other newly created files too). I added a new line at the end of every new file. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line The updated changes look reasonable to me. - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line Looks good. Thank you for the fix! - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line Marked as reviewed by joehw (Reviewer). Thanks for the update. A full test passed. - PR: https://git.openjdk.java.net/jdk/pull/4474
Re: RFR: 8268457: XML Transformer outputs Unicode supplementary character incorrectly to HTML [v3]
On Wed, 23 Jun 2021 07:34:06 GMT, Masanori Yano wrote: >> Hi all, >> >> Could you please review the 8268457 bug fixes? >> >> The problem is that ToHTMLStream applies processing for non-surrogate pairs >> to the surrogate pair. >> This fix changes the processing for non-surrogate pairs to the else >> condition. > > Masanori Yano has updated the pull request incrementally with one additional > commit since the last revision: > > remove unnecessally comments and add eof line Hi Masanori, you may now type /integrate in a new comment when you're ready. I'll then sponsor your change. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/4474
