On 5/3/2013 10:13 AM, Daniel Fuchs wrote:
Hi Joe,
I am not a JAXP expert - so I've been discovering most
of this code as I read it. It would be impossible for me
to assess whether there's some setFeature or setProperty
missing somewhere for instance. So with my limited
knowledge I have only these few remarks:
==========
1. XalanConstants.java: javadoc ORACLE_FEATURE_SERVICE_MECHANISM:
"instructs the implementation to use service mechanism to find
implementation"
That sounds a bit cryptic to my ears. Would something like:
"true: instruct an object to use service mechanism to
find a service implementation"
"false: instruct an object to skip service mechanism and
use the default implementation for that service"
work better? (disclaimer: I am not a native English speaker.)
Yes. Updated using the above.
==========
2.
<http://cr.openjdk.java.net/~joehw/jdk8/8011653/webrev/src/com/sun/org/apache/xerces/internal/impl/XMLDocumentFragmentScannerImpl.java.frames.html>
773 //JAXP 1.5 properties
774 if
(propertyId.startsWith(Constants.JAXPAPI_PROPERTY_PREFIX)) {
775 if (propertyId.equals(ACCESS_EXTERNAL_DTD))
776 {
777 fAccessExternalDTD = (String)value;
778 }
779 }
The first line (startsWith(...) seems useless.
When I saw this lines of code - I first thought there was a
bug - expecting ACCESS_EXTERNAL_DTD to be a suffix like
in the lines just before.
I was following the tradition of the original impl where properties such
as those from Xerces were grouped together. I was thinking there would
be other JAXP API properties in the future.
===========
3. Same remark than 2. with:
<http://cr.openjdk.java.net/~joehw/jdk8/8011653/webrev/src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.frames.html>
(lines 1643-1650)
===========
4. same class as above: what is the story with this
"Zephyr feature ignore-external-dtd" ?
Namely I'm puzzled as to why in one case we check only
the Zephyr feature, and in another case we check only the
Xerces feature.
Shouldn't both be checked?
I mean - are we sure that nobody can call something like:
PropertyManager propertyManager = ...;
propertyManager.setFeature(LOAD_EXTERNAL_DTD, false);
This is the result of integrating Zephyr (Sun's StAX impl) into the JDK
(since 1.6). Zephyr uses a PropertyManager rather than Xerces'
XMLComponentManager.
No, PropertyManager is internal to the implementation.
Thanks!
Joe
==========
-- daniel