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

+1 Good catch Naoto > On Mar 30, 2020, at 6:44 PM, Joe Wang wrote: > > > On 3/30/20 3:12 PM, naoto.s...@oracle.com wrote: >> Hi Joe, >> >> Much better and cleaner! One nit is that you could remove "docLocator != >> null &&" as instanceof checks null. > > Indeed, null check is removed: >

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

Looks good. Thanks for the update. Naoto On 3/30/20 3:44 PM, Joe Wang wrote: On 3/30/20 3:12 PM, naoto.s...@oracle.com wrote: Hi Joe, Much better and cleaner! One nit is that you could remove "docLocator != null &&" as instanceof checks null. Indeed, null check is removed:

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

On 3/30/20 3:12 PM, naoto.s...@oracle.com wrote: Hi Joe, Much better and cleaner! One nit is that you could remove "docLocator != null &&" as instanceof checks null. Indeed, null check is removed: http://cr.openjdk.java.net/~joehw/jdk15/8238183/webrev_03/ Thanks, Joe Naoto On 3/30/20

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

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

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

HI Joe, This version looks OK Best Lance > On Mar 30, 2020, at 5: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 >

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

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.

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

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

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

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: