stevedlawrence commented on a change in pull request #505:
URL: https://github.com/apache/daffodil/pull/505#discussion_r624122775



##########
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:
       Yep, your mental parse is correct.
   
   > But to any other content handler what was a(b(c)) in the minimized scope 
would now have become c(b(a)), no?
   
   I think the answer to this is in most cases it doesn't matter, and if it 
does it's up to the ContentHandler to decide. Here's my thought process:
   
   If the minimizedScope was a(b(c)), to Daffodil that reprents bindings in 
this order when converted to text:
   ```xml
   <a xmlns:a="1" xmlns:b="2" xmlns:c="3" />
   ```
   And so we will call startPrefixMapping in the order of a, b, c. This matces 
the order that the xmlns appears in the text representation. And I think that's 
all that matters. How a ContentHandler interprets these mapping and uses them 
for prefix lookups or converting to text is entirely up to them. Keep in mind 
they don't have to use NamespaceBindings. Maybe they use a stack of Maps or 
some complicated data structure. All we are really saying by calling 
startPrefixMapping is "this is the order we think the xmlns mappings appear in 
the text representation, you figure out the rest".
   
   And in most cases, order probably doesn't even matter. If they end up 
creating a NamespaceBinding that is c(b(a)) and do prefix lookups on that, the 
results will be exactly the same. This is because order doesn't matter in 
namespace bindings in relation to a single element (*see potential issue at 
bottom*). For example this is exactly the same as the above XML from a prefix 
lookup perspective:
   
   ```xml
   <a xmlns:c="3" xmlns:b="2" xmlns:c="1" />
   ```
   
   Regardless of the order of those mappings, the a, b, and c prefixes all map 
to the right uri. A ContentHandler could add those three to a NamespaceBinding 
(or any data structure of choosing) in any order and prefix lookups would work 
exactly the same (as long as they took precendece to any outer scope prefixes).
   
   The issue that potentially pops up is if a ContentHandler wants to convert 
the prefix mappings to XML text, like our DaffodilParseOutputStream thing does. 
In that case, we might care about the order of things, because it is no longer 
just about mappings (where order doesn't matter), but is now about outputing 
XML text (where order does kindof matter because of our tests). So our content 
handler is careful to outut xmlns text in the same order it recieves the 
mappings using the new NamespaceBinding.
   
   ---
   
   I think maybe the only case where order might matter with prefix look ups is 
if you had something like this:
   ```xml
   <pre:a xmlns:pre="foo" xmlns:pre="bar" />
   ```
   So you have the same prefix that maps to two diferent URIs. In that case, 
there are two potential namespace bindings depending on order:
   ```scala
   NamespaceBinding("pre", "foo", NamespaceBinding("pre, "bar", null))
   NamespaceBinding("pre", "bar", NamespaceBinding("pre, "foo", null))
   ```
   So depending on which NamespaceBinding is used, a prefix lookup of "pre" 
could return either "foo" or "bar". But it feels to me like redefining a prefix 
on the same element shouldn't even be allowed--order isn't supposed to matter 
in prefix mappings, so if prefix redefinition on a single element was allowed, 
then order would be the only other thing to pick a winner. I would guess this 
is illegal and isn't even something Daffodil would ever do.




-- 
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]


Reply via email to