Thanks Christoph, Daniel! I've updated the test, removing the comments.
Advertising
As for 80 chars line limit, or slightly longer than that is fine with
me. Given our codebase, it would be an enormous undertaking. I'm
therefore fine with taking care of only the extremely long ones or any
that impedes the reviews, that is, side-by-side view.
Best,
Joe
On 12/1/16, 2:25 AM, Daniel Fuchs wrote:
Hi Joe,
I agree with Christoph's comments below.
+1
best regards,
-- daniel
On 01/12/16 07:40, Langer, Christoph wrote:
Hi Joe,
to me this looks good.
Did you already remove the cleanups from
http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see
a lot of them any more...
A few minor points:
It seems you still have left some debugging code in
test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java:
59 while (xmlStreamReader.hasNext()) {
60 int event = xmlStreamReader.next();
61 if (event == XMLStreamConstants.START_ELEMENT) {
62 if
(xmlStreamReader.getLocalName().equals("body"))// && bMessage)
-> remove the && bMessage)
63 {
64 String elementText =
xmlStreamReader.getElementText();
65 //System.out.println("elementText=" +
elementText + "EndOfElementText");
-> the commented System.out.println statement should be removed at
all, I suggest
66 // fail if elementText contains "</body>"
67
Assert.assertTrue(!elementText.contains("</body>"), "Fail:
elementText contains </body>");
68 }
69 }
70 }
Other than that I'm wondering if the 80 chars line limit shall be
held which is not completely true in StreamReaderTest.java. But I
know how difficult that is, while keeping the code still readable.
Especially with the long speaking Class and method names ;-)
Best regards
Christoph
-----Original Message-----
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net]
On Behalf
Of Joe Wang
Sent: Mittwoch, 30. November 2016 22:21
To: core-libs-dev@openjdk.java.net
Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
corrupt
content when size of element is > 8192
Hi,
Please review an one-line fix and a bunch of cleanups.
The reported issue was caused by a missed setting when the
XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
involved 350 lines, the fix is really just the following:
XMLStreamReaderImpl.java:
@@ -605,11 +604,12 @@
...
+ fEntityScanner.registerListener(fScanner);
All other changes are cleanups, warnings. And BTW, warnings in the jaxp
repo have come down from 5230 in 2015 to 3265, a result of a bit of
cleanups here and there when we touch those classes. Still a long
way to
go, and it shows we may need to have a few dedicated patches.
JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/
Thanks,
Joe
