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: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/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-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
