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

[Langer, Christoph]

Hi Frank,

thanks, looks good :)

Best
Christoph

> -----Original Message-----
> From: Frank Yuan [mailto:frank.yuan at oracle.com]
> Sent: Donnerstag, 15. Dezember 2016 11:52
> To: Langer, Christoph <christoph.langer at sap.com>; 'Joe Wang'
> <huizhe.wang at oracle.com>
> Cc: core-libs-dev at 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.langer at 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-bounces at openjdk.java.net] On
> Behalf
> > > Of Joe Wang
> > > Sent: Mittwoch, 14. Dezember 2016 04:09
> > > To: Frank Yuan <frank.yuan at oracle.com>
> > > Cc: core-libs-dev at 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
> > > >
> > > >