[CC'ing the Nick Wellnhofer, the author of the commit that introduced
this issue]
So, I found what the problem with that commit is.
The commit removed the `xsltAttributeInternal` function which
distinguished between attributes added as a part of an attribute set
(which were not allowed to overwrite the attributes from the LREs,
literal result elements) and attributes added using <xsl:attribute/>
(which overwrite the attributes from the LREs). This is the removed
block - which was explicitly handling this case:
if (fromAttributeSet) {
/*
* This tries to ensure that xsl:attribute(s) coming
* from an xsl:attribute-set won't override attribute of
* literal result elements or of explicit xsl:attribute(s).
* URGENT TODO: This might be buggy, since it will miss to
* overwrite two equal attributes both from attribute sets.
*/
attr = xmlHasNsProp(targetElem, name, nsName);
if (attr != NULL)
return;
}
The assumption stated in the comment in xsltAttrListTemplateProcess()
/*
* Apply attribute-sets.
* The creation of such attributes will not overwrite any existing
* attribute.
*/
no longer holds because xsltAttribute now always overwrites the existing
attributes.
I don't think this removed block needs to be restored; instead, I think,
the xsltAttrListTemplateProcess() should process the attribute sets
first and then, iterating over LRE attributes, override the values if
there is a duplicate attribute. Instead of constructing the attribute
list itself, it can use xmlSetNsProp() which will either create a new
attribute, or set the value on an existing one. xsltAttribute() already
uses xmlSetNsProp() for that purpose.
Patch attached; fixes the test case I sent before. I am not sure how to
interpret the output of `make check` - if it causes any regressions or
not. Would appreciate a review from the maintainers.
Best regards,
Alexey.
On 2/11/19 4:34 PM, Alexey Neyman wrote:
Hi,
I noticed since version 1.1.30 of libxslt, the attributes on the
literal element are not merged correctly with the attributes specified
on the element itself. The language in the spec [1] ("7.1.4 Named
Attribute Sets") is as follows:
> Thus, for a literal result element, attributes from attribute sets
named in an xsl:use-attribute-sets attribute will be added first, in
the order listed in the attribute; next, attributes specified on the
literal result element will be added; finally, any attributes
specified by xsl:attribute elements will be added. Since adding an
attribute to an element replaces any existing attribute of that
element with the same name, this means that attributes specified in
attribute sets can be overridden by attributes specified on the
literal result element itself.
Below is a small test case that has two attribute sets ("as1", "as2";
used in that order) + attributes specified on the element itself
("element") and attributes specified via <xsl:attribute/> in the
content of the literal element. Per spec, the output of the test case
should be:
<bar a1="attr" a2="element" a3="as2" a4="as1"/>
With 1.1.30 and newer, the output is:
<bar a1="attr" a2="as2" a3="as2" a4="as1"/>
Before 1.1.30, the output was:
<bar a1="attr" a2="element" a3="as1" a4="as1"/>
The commit [2] that changed this behavior apparently fixed the order
of attributes merged from multiple attribute sets (see a3/a4
attributes above) but in doing so, broke the handling of the
attributes from the literal element (see a2 attribute above).
This, for example, breaks the docbook stylesheet customizations for
the titlepage (which auto-generate an XSLT stylesheet using both an
attribute sets and attributes on a literal element).
The test case:
---- a.xsl ----
<?xml version="1.0"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
version="1.0">
<xsl:template match="*">
<xsl:apply-templates/>
</xsl:template>
<xsl:attribute-set name="as1">
<xsl:attribute name="a1">as1</xsl:attribute>
<xsl:attribute name="a2">as1</xsl:attribute>
<xsl:attribute name="a3">as1</xsl:attribute>
<xsl:attribute name="a4">as1</xsl:attribute>
</xsl:attribute-set>
<xsl:attribute-set name="as2">
<xsl:attribute name="a1">as2</xsl:attribute>
<xsl:attribute name="a2">as2</xsl:attribute>
<xsl:attribute name="a3">as2</xsl:attribute>
</xsl:attribute-set>
<xsl:template match="foo">
<bar xsl:use-attribute-sets="as1 as2" a1="element" a2="element">
<xsl:attribute name="a1">attr</xsl:attribute>
</bar>
</xsl:template>
</xsl:stylesheet>
---- a.xml ----
<?xml version="1.0"?>
<foo/>
[1] https://www.w3.org/TR/1999/REC-xslt-19991116
[2]
https://gitlab.gnome.org/GNOME/libxslt/commit/05f70130433478c1075ce8b6fdc4c4dadf51d33e
Regards,
Alexey.
>From e72c507fac55a4a6d608b616231c9fd247dd18ac Mon Sep 17 00:00:00 2001
From: Alexey Neyman <sti...@att.net>
Date: Tue, 12 Feb 2019 00:56:50 -0800
Subject: [PATCH] LRE attribute must have precedence over attribute sets
Process attribute sets first. Then, use xmlSetNsProp to create
properties copied from the literal result elements (LREs) - which
overwrites the values from attribute sets.
Signed-off-by: Alexey Neyman <sti...@att.net>
---
libxslt/templates.c | 90 ++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 62 deletions(-)
diff --git a/libxslt/templates.c b/libxslt/templates.c
index 42559210..12a93951 100644
--- a/libxslt/templates.c
+++ b/libxslt/templates.c
@@ -645,7 +645,7 @@ xmlAttrPtr
xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
xmlNodePtr target, xmlAttrPtr attrs)
{
- xmlAttrPtr attr, copy, last;
+ xmlAttrPtr attr, copy;
xmlNodePtr oldInsert, text;
xmlNsPtr origNs = NULL, copyNs = NULL;
const xmlChar *value;
@@ -659,15 +659,29 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
ctxt->insert = target;
/*
- * Instantiate LRE-attributes.
+ * Apply attribute-sets.
+ * The creation of such attributes will not overwrite any existing
+ * attribute.
*/
- if (target->properties) {
- last = target->properties;
- while (last->next != NULL)
- last = last->next;
- } else {
- last = NULL;
- }
+ attr = attrs;
+ do {
+#ifdef XSLT_REFACTORED
+ if ((attr->psvi == xsltXSLTAttrMarker) &&
+ xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets"))
+ {
+ xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
+ }
+#else
+ if ((attr->ns != NULL) &&
+ xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets") &&
+ xmlStrEqual(attr->ns->href, XSLT_NAMESPACE))
+ {
+ xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
+ }
+#endif
+ attr = attr->next;
+ } while (attr != NULL);
+
attr = attrs;
do {
/*
@@ -702,34 +716,6 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
} else
value = xmlDictLookup(ctxt->dict, BAD_CAST "", 0);
- /*
- * Create a new attribute.
- */
- copy = xmlNewDocProp(target->doc, attr->name, NULL);
- if (copy == NULL) {
- if (attr->ns) {
- xsltTransformError(ctxt, NULL, attr->parent,
- "Internal error: Failed to create attribute '{%s}%s'.\n",
- attr->ns->href, attr->name);
- } else {
- xsltTransformError(ctxt, NULL, attr->parent,
- "Internal error: Failed to create attribute '%s'.\n",
- attr->name);
- }
- goto error;
- }
- /*
- * Attach it to the target element.
- */
- copy->parent = target;
- if (last == NULL) {
- target->properties = copy;
- last = copy;
- } else {
- last->next = copy;
- copy->prev = last;
- last = copy;
- }
/*
* Set the namespace. Avoid lookups of same namespaces.
*/
@@ -748,7 +734,11 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
} else
copyNs = NULL;
}
- copy->ns = copyNs;
+
+ // xmlSetNsProp will take care of the duplicates
+ copy = xmlSetNsProp(ctxt->insert, copyNs, attr->name, NULL);
+ if (copy == NULL)
+ goto error;
/*
* Set the value.
@@ -803,30 +793,6 @@ next_attribute:
attr = attr->next;
} while (attr != NULL);
- /*
- * Apply attribute-sets.
- * The creation of such attributes will not overwrite any existing
- * attribute.
- */
- attr = attrs;
- do {
-#ifdef XSLT_REFACTORED
- if ((attr->psvi == xsltXSLTAttrMarker) &&
- xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets"))
- {
- xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
- }
-#else
- if ((attr->ns != NULL) &&
- xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets") &&
- xmlStrEqual(attr->ns->href, XSLT_NAMESPACE))
- {
- xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
- }
-#endif
- attr = attr->next;
- } while (attr != NULL);
-
ctxt->insert = oldInsert;
return(target->properties);
--
2.19.1
_______________________________________________
xslt mailing list, project page http://xmlsoft.org/XSLT/
xslt@gnome.org
https://mail.gnome.org/mailman/listinfo/xslt
