Thank you all! I have pushed the change with the license header update. Frank
-----Original Message----- From: Joe Wang [mailto:[email protected]] 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 update. It looks good. The license header for output_html.properties can be updated as the follows: ########################################################################### # Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. ########################################################################### ## # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file # distributed with this work for additional information # regarding copyright ownership. The ASF licenses this file # to you under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # # http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. ## # # Specify defaults when method="html". These defaults use output_xml.properties # as a base. # Best, Joe On 12/15/16, 2:52 AM, Frank Yuan wrote: > 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:[email protected]] >> 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/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:[email protected]] On >>> Behalf >>> Of Joe Wang >>> Sent: Mittwoch, 14. Dezember 2016 04:09 >>> To: Frank Yuan<[email protected]> >>> Cc: [email protected] >>> 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 >>>> >>>>
