RFR: 8249867: xml declaration is not followed by a newline
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. ------------- Commit messages: - fix test, remove whitespace - 8249867: xml declaration is not followed by a newline Changes: https://git.openjdk.java.net/jdk/pull/2041/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8249867 Stats: 242 lines in 4 files changed: 193 ins; 1 del; 48 mod Patch: https://git.openjdk.java.net/jdk/pull/2041.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2041/head:pull/2041 PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 01:24:38 GMT, Joe Wang <joehw@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.
When setting the fDOMConfigProperties is the difference between simple values of "yes" and "no" and the values of DOMConstants.DOM3_EXPLICIT_TRUE/FALSE significant? ------------- Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 21:25:54 GMT, Roger Riggs <rriggs@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.
When setting the fDOMConfigProperties is the difference between simple values of "yes" and "no" and the values of DOMConstants.DOM3_EXPLICIT_TRUE/FALSE significant?
The DOMConfigProperties differentiate "Explicit" values from the default ones. DOM3_EXPLICIT_TRUE/FALSE has a string form: explicit:yes/explicit:no, while the default ones default:yes/default:no. In some cases (for some parameters), default values are handled differently from the explicit ones. In this case, it's not significant for the new property as to how it was processed. I merely followed the existing practice and mechanism to align with the existing parameters. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 01:24:38 GMT, Joe Wang <joehw@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.
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/dom3/DOMConstants.java line 131:
129: 130: // Corresponding System property 131: public static final String SP_IS_STANDALONE = "jdk.xml.isStandalone";
Should this system property key be described somewhere in the javadoc, as an Oracle implSpec? Otherwise, how the user would know this switch? ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 23:56:12 GMT, Naoto Sato <naoto@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.
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/dom3/DOMConstants.java line 131:
129: 130: // Corresponding System property 131: public static final String SP_IS_STANDALONE = "jdk.xml.isStandalone";
Should this system property key be described somewhere in the javadoc, as an Oracle implSpec? Otherwise, how the user would know this switch?
So far as with the current practice, we would document JDK specific properties in the Release Notes (refer to the Release Note sub-task for this particular property). We also created tutorials for the ones that are within a major topic (e.g. security). Plus, even if we'd want to add an implSpec, we wouldn't be able to since the DOM's license is quite restrictive. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 13 Jan 2021 00:17:57 GMT, Joe Wang <joehw@openjdk.org> wrote:
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/dom3/DOMConstants.java line 131:
129: 130: // Corresponding System property 131: public static final String SP_IS_STANDALONE = "jdk.xml.isStandalone";
Should this system property key be described somewhere in the javadoc, as an Oracle implSpec? Otherwise, how the user would know this switch?
So far as with the current practice, we would document JDK specific properties in the Release Notes (refer to the Release Note sub-task for this particular property). We also created tutorials for the ones that are within a major topic (e.g. security). Plus, even if we'd want to add an implSpec, we wouldn't be able to since the DOM's license is quite restrictive.
OK. Thanks for the explanation. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 13 Jan 2021 01:30:14 GMT, Naoto Sato <naoto@openjdk.org> wrote:
So far as with the current practice, we would document JDK specific properties in the Release Notes (refer to the Release Note sub-task for this particular property). We also created tutorials for the ones that are within a major topic (e.g. security). Plus, even if we'd want to add an implSpec, we wouldn't be able to since the DOM's license is quite restrictive.
OK. Thanks for the explanation.
Note: If such properties needed to be documented - they could also be documented as JDK implementation specific properties in the module-info file. This is what we have started doing for some of the properties supported by JNDI providers, for instance. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 01:24:38 GMT, Joe Wang <joehw@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.
Marked as reviewed by naoto (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
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. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2041/files - new: https://git.openjdk.java.net/jdk/pull/2041/files/3ab9fed9..a3591aa7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=00-01 Stats: 249 lines in 5 files changed: 233 ins; 5 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2041.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2041/head:pull/2041 PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 27 Jan 2021 06:33:01 GMT, Joe Wang <joehw@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/module-info.java line 82:
80: * 81: * <h3>jaxp.properties</h3> 82: * A system property can be specified in the {@code jaxp.properties} file to
I know this is covered in the tutorial, but perhaps add a section to module-info describing the jaxp.properties file so you do not have to leave the page to find out more details? ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 27 Jan 2021 11:39:29 GMT, Lance Andersen <lancea@openjdk.org> wrote:
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/module-info.java line 82:
80: * 81: * <h3>jaxp.properties</h3> 82: * A system property can be specified in the {@code jaxp.properties} file to
I know this is covered in the tutorial, but perhaps add a section to module-info describing the jaxp.properties file so you do not have to leave the page to find out more details?
The jaxp.properties file is described in detail in JAXP factories. I'll add a link to one of the factories. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 27 Jan 2021 06:33:01 GMT, Joe Wang <joehw@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
On Wed, 27 Jan 2021 15:34:26 GMT, Roger Riggs <rriggs@openjdk.org> wrote:
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")`
Will do. I'll see if we should do the same with the existing ones.
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)
will change to @code. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 27 Jan 2021 15:41:02 GMT, Roger Riggs <rriggs@openjdk.org> wrote:
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/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"
Fixed.
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.
Removed all cases. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Wed, 27 Jan 2021 06:33:01 GMT, Joe Wang <joehw@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 362:
360: // JDK specific property jdk-is-standalone 361: String p = SecuritySupport.getSystemProperty(DOMConstants.SP_IS_STANDALONE); 362: if (p == null || p.equals("")) {
Although I see it aligns with other locations, `p.isEmpty()` is better? ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
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: Updated the patch based on review comments. Refer to the previous reviews. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2041/files - new: https://git.openjdk.java.net/jdk/pull/2041/files/a3591aa7..0ed62318 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=01-02 Stats: 57 lines in 3 files changed: 14 ins; 8 del; 35 mod Patch: https://git.openjdk.java.net/jdk/pull/2041.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2041/head:pull/2041 PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 00:07:59 GMT, Joe Wang <joehw@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:
Updated the patch based on review comments. Refer to the previous reviews.
Hi Joe, The latest changes look fine ------------- Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 00:07:59 GMT, Joe Wang <joehw@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:
Updated the patch based on review comments. Refer to the previous reviews.
Marked as reviewed by rriggs (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 00:07:59 GMT, Joe Wang <joehw@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:
Updated the patch based on review comments. Refer to the previous reviews.
Marked as reviewed by naoto (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 00:07:59 GMT, Joe Wang <joehw@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:
Updated the patch based on review comments. Refer to the previous reviews.
I have just two cosmetic comments. Otherwise LGTM! src/java.xml/share/classes/module-info.java line 78:
76: * <h3>System Properties</h3> 77: * A property may have a corresponding System Property that has the same name 78: * except the prefix as shown above. A System Property should be set prior to
should that be "except for the prefix"? src/java.xml/share/classes/module-info.java line 188:
186: * </tbody> 187: * </table> 188: *
One question is whether the code samples in the table above should be escaped with {@code }. e.g.: <td> {@code first line of code;}<br> {@code second line of code;}<br> </td> ------------- Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 17:52:33 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
Updated the patch based on review comments. Refer to the previous reviews.
src/java.xml/share/classes/module-info.java line 78:
76: * <h3>System Properties</h3> 77: * A property may have a corresponding System Property that has the same name 78: * except the prefix as shown above. A System Property should be set prior to
should that be "except for the prefix"?
yes that would be clearer ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 18:40:55 GMT, Lance Andersen <lancea@openjdk.org> wrote:
src/java.xml/share/classes/module-info.java line 78:
76: * <h3>System Properties</h3> 77: * A property may have a corresponding System Property that has the same name 78: * except the prefix as shown above. A System Property should be set prior to
should that be "except for the prefix"?
yes that would be clearer
Changed. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 17:57:14 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Joe Wang has updated the pull request incrementally with one additional commit since the last revision:
Updated the patch based on review comments. Refer to the previous reviews.
src/java.xml/share/classes/module-info.java line 188:
186: * </tbody> 187: * </table> 188: *
One question is whether the code samples in the table above should be escaped with {@code }. e.g.:
<td> {@code first line of code;}<br> {@code second line of code;}<br> </td>
Looks better. ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
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 javadoc ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2041/files - new: https://git.openjdk.java.net/jdk/pull/2041/files/0ed62318..44a568b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2041&range=02-03 Stats: 15 lines in 1 file changed: 0 ins; 0 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/2041.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2041/head:pull/2041 PR: https://git.openjdk.java.net/jdk/pull/2041
On Fri, 29 Jan 2021 18:24:59 GMT, Joe Wang <joehw@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 javadoc
Marked as reviewed by dfuchs (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
On Tue, 12 Jan 2021 01:24:38 GMT, Joe Wang <joehw@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.
This pull request has now been integrated. Changeset: 69ee314b Author: Joe Wang <joehw@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/69ee314b Stats: 479 lines in 5 files changed: 428 ins; 2 del; 49 mod 8249867: xml declaration is not followed by a newline Reviewed-by: rriggs, naoto, lancea, dfuchs ------------- PR: https://git.openjdk.java.net/jdk/pull/2041
participants (5)
-
Daniel Fuchs
-
Joe Wang
-
Lance Andersen
-
Naoto Sato
-
Roger Riggs