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

Frank Yuan frank.yuan at oracle.com
Mon Dec 19 03:17:26 UTC 2016 

Thank you all! I have pushed the change with the license header update.

Frank

-----Original Message-----
From: Joe Wang [mailto:huizhe.wang at oracle.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,

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