Hi Joe, I finally got back to this one and pushed the change with some minor whitespace diffs.
Thanks Christoph > -----Original Message----- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Montag, 28. November 2016 20:06 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > namespace-unaware SAX input yields a different result than namespace- > unaware DOM input > > Hi Christoph, > > The changes look good. I also run the other tests (smoke test and etc.), > and they all passed. > > Best, > Joe > > On 11/25/16, 4:25 AM, Langer, Christoph wrote: > > Hi Joe, > > > > when trying to finish this up, I had a closer look again. > > > > I found out that it's necessary to look at attributes as well and handle > > the case > where namespace prefixes are used. > > > > Here is a new version of the changeset that passes all jaxp jtreg tests: > > http://cr.openjdk.java.net/~clanger/webrevs/8169631.1/ > > > > Now I would also throw out prefixes in localName if we don't have > namespace support - for both, elements and attributes (in > SAX2DTM2.startElement()). > > > > I also removed some qname handling in DOM2SAX as it is not needed with > my changes to SAX2DTM2 anymore. > > > > The test case was adopted. > > > > Thanks& best regards > > Christoph > > > >> -----Original Message----- > >> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >> Sent: Dienstag, 22. November 2016 20:11 > >> To: Langer, Christoph<christoph.lan...@sap.com> > >> Cc: core-libs-dev@openjdk.java.net > >> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >> namespace-unaware SAX input yields a different result than namespace- > >> unaware DOM input > >> > >> Hi Christoph, > >> > >> Once you're able to run all tests, feel free to push the changeset. > >> Frank has fixed the Smoke test. > >> > >> Thanks, > >> Joe > >> > >> On 11/18/16, 3:37 PM, Joe Wang wrote: > >>> Hi Christoph, > >>> > >>> Thanks for explaining the customer's dilemma with regard to their > >>> legacy process. > >>> > >>> The testcase I sent you was extracted from an internal SQE smoke test. > >>> I agree with your analysis, the 'golden' file which has been in there > >>> for over 10 years turns out to be wrong and needs to be updated. > >>> > >>> To fix this issue, we need to get that test fixed, and the check-in of > >>> your patch and that of the test need needs to happen simultaneously. > >>> Would you mind wanting for me to go through an internal process to get > >>> a patch ready, then we can check in almost at the same time? > >>> > >>> Best, > >>> Joe > >>> > >>> On 11/18/16, 2:51 PM, Langer, Christoph wrote: > >>>> Hi Joe, > >>>> > >>>> thanks for the feedback. > >>>> > >>>> I've now understood the testcase that you've sent over and the reason > >>>> that it is reporting failure after my fix is that the output of its > >>>> transform operation is rather correct now. And before it was wrong. :) > >>>> The test is comparing the actual result to a "golden" result file in > >>>> the end and both of these were not looking healthy so far. The reason > >>>> is that your test is using a namespace unaware SAX Parser as input. > >>>> With the current JDK XALAN, you could already modify your smoketest > >>>> to use a namespace aware parser. > >>>> > >>>> E.g. replace lines > >>>> > >>>> 82 // Use the JAXP way to get an XMLReader > >>>> 83 XMLReader reader = > >>>> SAXParserFactory.newInstance().newSAXParser().getXMLReader(); > >>>> > >>>> with > >>>> > >>>> 82 // Use the JAXP way to get an XMLReader > >>>> 83 SAXParserFactory spf = SAXParserFactory.newInstance(); > >>>> 84 spf.setNamespaceAware(true); > >>>> 85 XMLReader reader = spf.newSAXParser().getXMLReader(); > >>>> > >>>> ...and you would already get correct results that also DOM input or > >>>> Stream Input would yield. > >>>> > >>>> So, are there other concerns/issues with this fix? Do you want me to > >>>> include a transformation operation like the one that your SmokeTest > >>>> does to TransformerTest which would illustrate the problem with > >>>> namespace unaware SAX input data? > >>>> > >>>> Best regards > >>>> Christoph > >>>> > >>>>> -----Original Message----- > >>>>> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >>>>> Sent: Freitag, 18. November 2016 05:53 > >>>>> To: Langer, Christoph<christoph.lan...@sap.com> > >>>>> Cc: core-libs-dev@openjdk.java.net > >>>>> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >>>>> namespace-unaware SAX input yields a different result than namespace- > >>>>> unaware DOM input > >>>>> > >>>>> > >>>>> > >>>>> On 11/14/16, 11:43 PM, Langer, Christoph wrote: > >>>>>> Hi Joe, > >>>>>> > >>>>>> thanks for looking. > >>>>>> > >>>>>> Can you let me know which smoke test is failing? I didn't see > >>>>>> issues so far - I > >>>>> was merely running the jtreg unittests for transformer. > >>>>> > >>>>> I sent the test to your mailbox. > >>>>>> I stepped back from replacing Vector with ArrayList for > >>>>>> m_prefixMappings > >>>>> because the code is using methods indexOf() with a start index and > >>>>> setSize() for > >>>>> which ArrayList has no direct matchings. One could, for sure, add > >>>>> some other > >>>>> coding, e.g. use ArrayList's subList() method for the index based > >>>>> search - but I > >>>>> wouldn't want to run the risk of adding a regression here just > >>>>> because I > >>>>> modified the code and did not well test it. But if you insist, I > >>>>> could have another > >>>>> look. > >>>>> > >>>>> Ok, that's fine. subList would do, but setSize may need a bit more > >>>>> work. > >>>>> > >>>>> Best, > >>>>> Joe > >>>>>> Best regards > >>>>>> Christoph > >>>>>> > >>>>>>> -----Original Message----- > >>>>>>> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >>>>>>> Sent: Dienstag, 15. November 2016 03:23 > >>>>>>> To: Langer, Christoph<christoph.lan...@sap.com> > >>>>>>> Cc: core-libs-dev@openjdk.java.net > >>>>>>> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >>>>>>> namespace-unaware SAX input yields a different result than > namespace- > >>>>>>> unaware DOM input > >>>>>>> > >>>>>>> Hi Christoph, > >>>>>>> > >>>>>>> Not all tests have finished yet, but there's at least one failure > >>>>>>> in the > >>>>>>> smoke test. I'll get to the details when I have time. > >>>>>>> > >>>>>>> Any reason why m_prefixMappings can not be replaced with > ArrayList? > >>>>>>> > >>>>>>> Thanks, > >>>>>>> Joe > >>>>>>> > >>>>>>> On 11/14/16, 6:10 AM, Langer, Christoph wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> please review this fix for bug 8169631. > >>>>>>>> > >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8169631 > >>>>>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169631.0/ > >>>>>>>> > >>>>>>>> When XALAN is handling namespace unaware input, it behaves > >>>>>>>> differently > >>>>>>> while using SAX input compared to DOM input. > >>>>>>>> With both input source types, the class > >>>>>>> com.sun.org.apache.xml.internal.dtm.ref.sax2dtm.SAX2DTM2 > converts > >> SAX > >>>>>>> input into a DTM representation for processing by the XALAN > >>>>>>> transformer. > >>>>> Its > >>>>>>> method startElement takes URI, localname and qName as attribute. > >>>>>>> In case > >>>>> of > >>>>>>> missing feature namespaces, startElement and localname can be > empty. > >>>>>>> However, the function uses the localname value for the call to > >>>>>>> m_expandedNameTable.getExpandedTypeID() and further processing. > In > >>>>>>> the > >>>>>>> case where only qName has data, this leads to issues. > >>>>>>>> When using DOM input, the class > >>>>>>> com.sun.org.apache.xalan.internal.xsltc.trax.DOM2SAX converts the > >> DOM > >>>>> input > >>>>>>> into SAX input. In the case of empty localname, it fills localname > >>>>>>> with qname > >>>>>>> data. See method getLocalName() [1], called by parse() [2]. > >>>>>>>> When directly using SAX input, the SAX parser calls the > >>>>>>>> startElement() > >>>>>>> function on XALAN's handler with empty uri and localname - which > >>>>>>> seems > >>>>>>> correct, as per the spec. > >>>>>>>> Both paths end up in SAX2DTM2's startElement(). So I suggest to > >>>>>>>> change > >>>>> this > >>>>>>> method to handle the case when uri and localname are empty and > >>>>>>> then set > >>>>>>> qname as localname. Maybe one should even change DOM2SAX's > >>>>>>> getLocalName handling to not fill localname with qname in case it > >>>>>>> is empty > >>>>>>> after SAX2DTM was changed.. > >>>>>>>> Generally, JavaDoc for SAXSource says that "Attempting to > >>>>>>>> transform an > >>>>> input > >>>>>>> source that is not generated with a namespace-aware parser may > >>>>>>> result in > >>>>>>> errors." But why not fix some of these :) > >>>>>>>> Furthermore I did some cleanups in the code. > >>>>>>>> > >>>>>>>> Thanks and best regards > >>>>>>>> Christoph > >>>>>>>> > >>>>>>>> [1] > >> > http://hg.openjdk.java.net/jdk9/dev/jaxp/file/71558b38bad7/src/java.xml/shar > >>>>> > >> > e/classes/com/sun/org/apache/xalan/internal/xsltc/trax/DOM2SAX.java#l139 > >>>>>>>> [2] > >> > http://hg.openjdk.java.net/jdk9/dev/jaxp/file/71558b38bad7/src/java.xml/shar > >>>>> > >> > e/classes/com/sun/org/apache/xalan/internal/xsltc/trax/DOM2SAX.java#l279 > >>>>>>>> [3] > >> > https://docs.oracle.com/javase/8/docs/api/javax/xml/transform/sax/SAXSource > >>>>>>> .html
