Hi Frank, thanks, looks good :)
Best Christoph > -----Original Message----- > From: Frank Yuan [mailto:frank.y...@oracle.com] > Sent: Donnerstag, 15. Dezember 2016 11:52 > To: Langer, Christoph <christoph.lan...@sap.com>; 'Joe Wang' > <huizhe.w...@oracle.com> > Cc: core-libs-dev@openjdk.java.net > Subject: RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work > anymore and JDK-8114834 LSSerializerImpl always serializes an entity > reference node to" &entityName;" even if "entities" property is false > > Hi Christoph > > Thank you for the review! > > Please check http://cr.openjdk.java.net/~fyuan/8087303_8114834/webrev.01/. > > I have updated the code as your comments except output_html.properties, I am > not sure for the license of this file. > > To Joe > Could you confirm Christoph's comment for output_html.properties? > > > Thanks > Frank > > > -----Original Message----- > > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > > Subject: RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work > anymore and JDK-8114834 LSSerializerImpl always > serializes an > > entity reference node to" &entityName;" even if "entities" property is false > > > > Hi Frank, > > > > this is awesome. > > > > Some minor things I saw while looking through the webrev: > > > > > src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/runtime/ > AbstractTranslet.java > > Update copryright year > > Remove: > > 20 /* > > 21 * $Id: AbstractTranslet.java,v 1.6 2006/06/19 19:49:03 spericas Exp $ > > 22 */ > > > > > src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/Tran > sformerImpl.java > > Remove: > > 20 /* > > 21 * $Id: TransformerImpl.java,v 1.10 2007/06/13 01:57:09 joehw Exp $ > > 22 */ > > > > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/Seriali > zerBase.java > > 462 protected boolean isInEntityRef() > > 463 { > > Put the brace on line 462 as well > > > > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHT > MLStream.java > > -> sort import statements > > Method > > 773 public void startElement( > > -> use SAXException without package name. It is imported on top. This can be > done in various places throughout the file. > > 780 //will add extra one if having namespace but no matter > > -> space between // and will... > > 821 if ((null != elemContext.m_elementName) > > -> For me it reads better if it were if ((elemContext.m_elementName != null) > > > > 1971 private void initToHTMLStream() > > 1972 { > > 1973 // m_elementDesc = null; > > 1974 m_isprevblock = false; > > 1975 m_inDTD = false; > > 1976 // m_isRawStack.clear(); > > 1977 m_omitMetaTag = false; > > 1978 m_specialEscapeURLs = true; > > 1979 } > > -> I guess you could remove the commented statements > > > > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStre > am.java > > 116 protected boolean m_ispreserveSpace = false; > > 117 > > 118 > > -> remove one empty line (117) > > > > 1894 m_ispreserve = false; > > 1895 > > 1896 > > 1897 > > -> newly inserted lines 1896 and 1897 should be removed > > > > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/output > _html.properties > > -> maybe the Oracle copyright header can be inserted and the "$Id: > output_html.properties..." part can be removed? > > > > Best regards > > Christoph > > > > > -----Original Message----- > > > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf > > > Of Joe Wang > > > Sent: Mittwoch, 14. Dezember 2016 04:09 > > > To: Frank Yuan <frank.y...@oracle.com> > > > Cc: core-libs-dev@openjdk.java.net > > > Subject: Re: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not > > > work > > > anymore and JDK-8114834 LSSerializerImpl always serializes an entity > > > reference node to" &entityName;" even if "entities" property is false > > > > > > Hi Frank, > > > > > > Thanks for the diligent work! I think we've made a great improvement > > > over the PrettyPrint (tremendous ;-) ) > > > > > > Could you look into extracting the XML literal strings in the test into > > > plain files (similar to the other 'gold' files)? What I'm hoping to see > > > is that they'd look easily prettier in a text editor, and for that > > > matter, the original xml files were a mess. > > > > > > The tests set PrettyPrint, and by default for html. It would be good to > > > test the cases when it's turned off, that would help verify the > > > non-pretty format was not changed. > > > > > > Thanks, > > > Joe > > > > > > On 12/13/16, 6:55 AM, Frank Yuan wrote: > > > > Hi all > > > > > > > > Webrev http://cr.openjdk.java.net/~fyuan/8087303_8114834/webrev.00/ > is > > > for JDK-8087303 and JDK-8114834, I have to combine the fix > > > > because there is some interaction between them. > > > > Bugs: https://bugs.openjdk.java.net/browse/JDK-8087303 > > > https://bugs.openjdk.java.net/browse/JDK-8114834 > > > > > > > > Besides fixed some issues in xml serializer, I made the following > > > > changes > in > > > this patch: > > > > 1. refined the handling for whitespace text > > > > 2. added support for xml:space attribute > > > > 3. changed the default indentation to 4 > > > > 4. refined the handling for HTML block element and inline element > > > > > > > > Would you like to have a look at this review? > > > > > > > > Thanks, > > > > > > > > Frank > > > > > > > >
