RFR: 8249867: xml declaration is not followed by a newline [v2]

Roger Riggs rriggs at openjdk.java.net
Wed Jan 27 15:52:44 UTC 2021


On Wed, 27 Jan 2021 06:33:01 GMT, Joe Wang <joehw at openjdk.org> wrote:

>> Please review a patch to add an explicit control over whether a newline should be added after the XML header. This is done by adding a DOM LSSerializer property "jdk-is-standalone" and System property "jdk.xml.isStandalone". 
>> 
>> This change addresses an incompatibility introduced during 7u4 as an update to Xalan 2.7.1.
>
> Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update: add javadoc for impl specific features and properties in module-info; update the patch accordingly.

src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/dom3/LSSerializerImpl.java line 639:

> 637:                 if (state) {
> 638:                     fDOMConfigProperties.setProperty(DOMConstants.S_JDK_PROPERTIES_NS
> 639:                             + DOMConstants.S_IS_STANDALONE, "yes");

Multiple concatenations of the same strings, seems awkward at best and perhaps a maintenance burden.
Would it make sense to declare them as static strings?
Though in this case, it could be a single call to setProperty with the value being `(state ? "yes" : "no")`

src/java.xml/share/classes/module-info.java line 58:

> 56:  * The prefix for System Properties:
> 57:  * <pre>
> 58:  *     {@systemProperty jdk.xml.}

The use of {@systemProperty...} seems inappropriate, it should be used to refer to specific properties. (IMHO)

src/java.xml/share/classes/module-info.java line 64:

> 62:  * A name may consist of one or multiple words that are case-sensitive.
> 63:  * All letters of the first word shall be in lowercase, while the first letter of
> 64:  * each subsequent word capitalized.

capitalized -> "is capitalized"

src/java.xml/share/classes/module-info.java line 91:

> 89:  * <h3 id="ScopeAndOrder">Scope and Order</h3>
> 90:  * The {@link javax.xml.XMLConstants#FEATURE_SECURE_PROCESSING} feature
> 91:  * (hereafter referred to as FSP) is required for XML processors including DOM,

I think it would be more readable to use "secure processing" instead of "FSP".
I.e.  when secure processing is set..., etc.

src/java.xml/share/classes/module-info.java line 103:

> 101:  * <p>
> 102:  * Properties specified in the jaxp.properties file affect all invocations of
> 103:  * the JDK and JRE, and will override their default values, or those that may

We've phased out the "JRE", so it can be dropped here.

src/java.xml/share/classes/module-info.java line 107:

> 105:  * <p>
> 106:  * System properties, when set, affect the invocation of the JDK and JRE, and
> 107:  * override the default settings or that set in jaxp.properties, or those that

"that set" -> 'that are set"

-------------

PR: https://git.openjdk.java.net/jdk/pull/2041


More information about the core-libs-dev mailing list