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 christoph.langer at sap.com
Thu Dec 15 07:48:17 UTC 2016


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/TransformerImpl.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/SerializerBase.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/ToHTMLStream.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/ToStream.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
> >
> >


More information about the core-libs-dev mailing list