RFR [15/java.xml] 8238183: SAX2StAXStreamWriter cannot deal with comments prior to the root element

[naoto.sato at oracle.com]

Hi Joe,

Much better and cleaner! One nit is that you could remove "docLocator != 
null &&" as instanceof checks null.

Naoto

On 3/30/20 2:56 PM, Joe Wang wrote:
> Hi Naoto,
> 
> Thanks for the review!
> 
> I refactored the code around getting the XML version and encoding from 
> the locator to get rid of the CCE. The impls with EventWriter and 
> StreamWriter share a base class. All three classes are therefore 
> refactored. No material change to the EventWriter impl. Two fields, 
> xmlVersion and encoding, are added since I expect they will be needed 
> later when we work on fixing the declaration. Note that, as of the 
> current impl, encoding is not referenced in the StreamWriter impl, which 
> is part of the issue in transforming the declaration (JDK-8241711).
> 
> Here's webrev version 2:
> http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_02/
> 
> -Joe
> 
> On 3/30/20 11:23 AM, naoto.sato at oracle.com wrote:
>> Hi Joe,
>>
>> Can writeStartDocument() be simplified by checking "docLocator 
>> instanceof Locator2"? This way it won't need to catch CCE and issue 
>> no-arg version.
>>
>> Otherwise looks good to me.
>>
>> Naoto
>>
>> On 3/30/20 11:02 AM, Joe Wang wrote:
>>> Hi,
>>>
>>> Please review a fix for the StAXResult impl. The issue was that it 
>>> output comment prior to the declaration, resulting in an invalid XML 
>>> document. The patch focuses on fixing this issue, but it does not 
>>> cover other issues the StAXResult impl may have (e.g. JDK-8241711).
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8238183
>>> webrev: http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev/
>>>
>>> Thanks,
>>> Joe
>>>
>