Hi Naoto and all,

Could you please take a look at the changes to the resource files for tests of JDK-8172365 & JDK-8210408?

Current webrevs:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v04/

Previous:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v03/

Thanks,
Joe

On 11/16/18, 2:27 PM, Joe Wang wrote:
Hi Daniel,

On 11/16/18, 10:32 AM, Daniel Fuchs wrote:
Hi Joe,

1. It looks as if the parser will not complain if the root element
   is seen twice - and it looks as if it is a supported feature
   since the previous implementation also had a counter.

   Should this be tested?
   If this is a feature and multiple roots are supported
   then the new impl might have an issue with allowing only
   one comment (maybe).

Removed "properties" from the ALLOWED_ELEMENTS so that it will report an error on properties inside properties, and added a test (IllegalElement.xml).

2. The rootElm qName is now set at two locations. Is the overridden
   method below still needed? Could it have been used instead of
   introducing a new startDTD method?

 212     @Override
213 public void notationDecl(String name, String publicId, String systemId) throws SAXException {
 214         rootElm = name;
 215     }

Removed the method override. startDTD is the correct place to accomplish this.

Updated webrev:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v03/

Previous:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v02/

Best regards,
Joe


best regards,

-- daniel

On 15/11/2018 20:26, Joe Wang wrote:
Hi Daniel,

I deleted the endDTD method. It could have been used to signal the end of DTD parsing and therefore serve as a validation point. However, the parser would have thrown Exceptions if a DTD parsing wasn't completed properly, that seems to make an endDTD check unnecessary, at least as the current parser impl goes.

The small footprint parser has its problems, for example, not providing specific error messages when things go wrong. But since this is a Properties usage only, it serves the purpose well enough.

Updated webrev:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v02/

Previous:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v01/

Best regards,
Joe

On 11/14/18, 2:32 AM, Daniel Fuchs wrote:
Hi Joe,

I do not see where the new DTDHandler::endDTD methos is called.
Is that an oversight or is there more magic?

best regards,

-- daniel

On 14/11/2018 05:18, Joe Wang wrote:
Hi,

Please review a patch to bring the small footprint parser for java.util.Properties compliant with the specification.

JBS: https://bugs.openjdk.java.net/browse/JDK-8213325
webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev/

Thanks,
Joe


Reply via email to