You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2025/01/23 17:52:24 UTC
[PR] Demonstrate bug in namespacx handling when including document from default namespace [maven-xinclude-extension]
elharo opened a new pull request, #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21
#20
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21#issuecomment-2753588579
@elharo do you want to have a look ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21#discussion_r2014903731
##########
src/main/java/org/apache/maven/xinclude/stax/XIncludeStreamReader.java:
##########
@@ -266,9 +275,25 @@ private void processInclude() throws XMLStreamException {
p = np instanceof Element ? (Element) np : null;
}
if (!Objects.equals(curLang, impLang)) {
- resNode.setAttributeNS(XML_NS_URI, "xml:lang", impLang);
+ resNode.setAttributeNS(XML_NS_URI, XML_NS_PREFIX + ":lang", impLang);
+ }
+ resNode.setAttributeNS(XML_NS_URI, XML_NS_PREFIX + ":base", input.getSystemId());
+
+ NamespaceContext context =
+ this.contextStack.peek().getReader().getNamespaceContext();
+ Map<String, String> namespaces = getAllNamespaces(resNode.getParentNode());
+ for (Map.Entry<String, String> entry : namespaces.entrySet()) {
+ String prefix = entry.getKey();
+ String uri = entry.getValue();
+ if (context != null
+ && context.getNamespaceURI(prefix) != null
+ && !uri.equals(context.getNamespaceURI(prefix))) {
+ resNode.setAttributeNS(
+ XMLNS_ATTRIBUTE_NS_URI,
+ prefix.isEmpty() ? XMLNS_ATTRIBUTE : XMLNS_NS_PREFIX + ":" + prefix,
Review Comment:
This would be easier to follow with string constants:
```
prefix.isEmpty() ? "xmlns" : "xmlns:" + prefix,
```
##########
src/main/java/org/apache/maven/xinclude/stax/XIncludeStreamReader.java:
##########
@@ -283,7 +308,7 @@ private void processInclude() throws XMLStreamException {
}
}
if (setXmlId != null) {
- resNode.setAttributeNS(XML_NS_URI, "xml:id", setXmlId);
+ resNode.setAttributeNS(XML_NS_URI, XML_NS_PREFIX + ":id", setXmlId);
Review Comment:
This would be clearer as just "xml:id".
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21#issuecomment-2753986774
let me see if I can find a minute
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet closed pull request #21: Fix namespace handling when including document from default namespace
URL: https://github.com/apache/maven-xinclude-extension/pull/21
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Demonstrate bug in namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21#issuecomment-2611782201
> #20
>
> This new test should pass but fails. The included document should not necessarily assume that its default namespace is the same as the document it's being merged into.
A code style fix would be welcome to actually show that the test fail.
However, I do think the test is wrong. There's a subtle difference between having no namespace specified (i.e. an element being in the default namespace), and an element with a `xmlns=""` specified.
For example, if you change the disclaimer to be:
```
<disclaimer xmlns="">
<p>The opinions represented herein represent those of the individual
and should not be interpreted as official policy endorsed by this
organization.</p>
</disclaimer>
```
then the test suddenly works.
So if the included element has no namespace specified, it does inherit the one from the including document.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Demonstrate bug in namespace handling when including document from default namespace [maven-xinclude-extension]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #21:
URL: https://github.com/apache/maven-xinclude-extension/pull/21#issuecomment-2612311332
Sorry, no. There is no difference between having no namespace specified and having a xmlns="" specified. The namespaces spec is explicit on this point: "The attribute value in a default namespace declaration MAY be empty. This has the same effect, within the scope of the declaration, of there being no default namespace."
It might be that the code is looking at the xmlns attributes when it should be looking at the namespaces in scope infoset property. How that's represented in StAX I'd have to dig in to the code to figure out.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org