Thanks Aleksej. The change looks good.
Best,
Joe
On 5/12/2016 10:00 AM, Aleks Efimov wrote:
Hi Joe,
Thanks for your comments and suggestions. New webrev can be found at
this location:
http://cr.openjdk.java.net/~aefimov/8145974/9/01
Answers are inlined.
Best Regards,
Aleksej
On 12/05/16 07:51, huizhe wang wrote:
Hi Aleksej,
The change looks good overall. It may be better to replace the name
"writeCodePoint" with "writeCharRef" or "writeEscaped".
Replaced with 'writeCharRef'
Doing the "isSurrogatePair" check in place of the call for
writeSurrogatePair may be more descriptive and readable as well, that
is:
Agree. writeSurrogatePair is replaced with isSurrogatePair in-place
check - was a leftover from previous version of the fix.
If you do that, you wouldn't need the method "writeSurrogatePair".
"writeSurrogatePair" was removed
For the test, it may be good to call writer.close() at the end of the
test. Also, would the content read is the same (vs contains) as the
expectedContent?
"writer.close()" was added and 'contains' was replaced with testng's
assertEquals.
Thanks,
Joe
On 5/11/2016 4:30 PM, Aleks Efimov wrote:
Hello,
Please, help to review the fix for XMLStreamWriter bug [1]:
XMLStreamWriter incorrectly writes surrogate pairs into pair of
invalid character references.
For example: "\ud83d\ude0a" is transformed into "��".
It should be one character reference "😊" instead.
The proposed patch fixes the XMLStreamWriterImpl to correctly
process surrogate pairs:
http://cr.openjdk.java.net/~aefimov/8145974/9/00
The build with fix applied was tested with JTREG and JCK xml tests -
no related issues detected.
Aleksej
[1] https://bugs.openjdk.java.net/browse/JDK-8145974
