stevedlawrence commented on a change in pull request #505:
URL: https://github.com/apache/daffodil/pull/505#discussion_r624047759
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/infoset/SAXInfosetOutputter.scala
##########
@@ -145,18 +145,11 @@ class SAXInfosetOutputter(xmlReader:
DFDL.DaffodilParseXMLReader,
private def doStartPrefixMapping(diElem: DIElement, contentHandler:
ContentHandler): Unit = {
val (nsbStart: NamespaceBinding, nsbEnd: NamespaceBinding) =
getNsbStartAndEnd(diElem)
var n = nsbStart
- var mappingsList: Seq[(String, String)] = Seq()
while (n.ne(nsbEnd) && n.ne(null) && n.ne(scala.xml.TopScope)) {
val prefix = if (n.prefix == null) "" else n.prefix
val uri = if (n.uri == null) "" else n.uri
- // we generate a list here by prepending so can build the prefixMapping
in the
- // same order as it is in minimizedScope when we call
startPrefixMapping; we do this because
- // getPrefix in the contentHandler is order dependent
- mappingsList +:= (prefix, uri)
- n = n.parent
- }
- mappingsList.foreach{ case (prefix, uri) =>
contentHandler.startPrefixMapping(prefix, uri)
+ n = n.parent
Review comment:
Hmm, I'm not sure. Imagine our namespace prefixes for an element were
this (in no particular order):
* ``xmlns:a="foo"``
* ``xmlns:b="bar"``
* ``xmlns:c="baz"``
With the new sorting, minizedScope for the above would like this:
```scala
NamespaceBinding("a", "foo", NamespaceBinding("b", "bar",
NamespaceBinding("c","baz", null)))
```
When we do a toString on this (e.g. what XMLTextInfoOutputter and others
do), this will be output to the string:
```
xmlns:a="foo" xmlns:b="bar" xmlns:c="baz"
```
So everything is in the expected sorted order.
But in the above before thsi change, we called startPrefixMapping in the
reverse order than they were in minimizedScope. So with the above
NamespaceBinding, I think we would call this before:
```scala
contentHandler.startPrefixMapping("c", "baz")
contentHandler.startPrefixMapping("b", "bar")
contentHandler.startPrefixMapping("a", "foo")
```
Which means the ContentHandler got the mappings in the reverse order than
they should show up in the XML.
I think the DaffodilParseOutputStreamContentHandler expected this reverse
order and then effecitvely re-reversed things when it added them to the
currentElement/activePrefixMapping so things showed up correct. But I think
maybe that was incorrect? With these changes, startPrefixMapping is called in
the same order as the minimizedScope, and then the new logic in the
DaffodilParseOutputStreamContentHandler ensures things are written to the
Writer in the correct order.
So I *think* this actually fixes the order that we call startPrefixMapping
to match minimzedScope? And perhaps they would have showed up backwards to
other ContentHandlers?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
