From: Kasimier T. Buchcik <kbuchcik@src.gnome.org>
Date: Thu, 1 Jun 2006 12:03:21 +0000 (+0000)
Subject: Next step in the refactored code: enhanced xsl:attribute; enhanced
X-Git-Tag: v1.1.28~244
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=de35a610d934a0269f3e7a1256ba63cf4db100eb;p=platform%2Fupstream%2Flibxslt.git

Next step in the refactored code: enhanced xsl:attribute; enhanced

* libxslt/attributes.c libxslt/variables.c
  libxslt/transform.c libxslt/xslt.c:
  Next step in the refactored code: enhanced xsl:attribute;
  enhanced xsltCopyProp.
  Added the creation of an XPath cache in
  xsltNewTransformContext().
---

diff --git a/ChangeLog b/ChangeLog
index db4cd52..a48964f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Thu Jun  1 13:58:19 CEST 2006 Kasimier Buchcik <libxml2-cvs@cazic.net>
+
+	* libxslt/attributes.c libxslt/variables.c
+	  libxslt/transform.c libxslt/xslt.c:
+	  Next step in the refactored code: enhanced xsl:attribute;
+	  enhanced xsltCopyProp.
+	  Added the creation of an XPath cache in
+	  xsltNewTransformContext().
+
 Wed May 31 22:32:44 CEST 2006 Kasimier Buchcik <libxml2-cvs@cazic.net>
 
 	* libxslt/transform.c: Fixed a difference in processing of
diff --git a/libxslt/attributes.c b/libxslt/attributes.c
index 4815b41..785e654 100644
--- a/libxslt/attributes.c
+++ b/libxslt/attributes.c
@@ -632,7 +632,7 @@ xsltResolveStylesheetAttributeSet(xsltStylesheetPtr style) {
  * @node:  the node in the source tree.
  * @inst:  the xsl:attribute element
  * @comp:  precomputed information
- * @fromset:  the attribute comes from an attribute-set
+ * @fromAttributeSet:  the attribute comes from an attribute-set
  *
  * Process the xslt attribute node on the source node
  */
@@ -640,59 +640,113 @@ static void
 xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
                       xmlNodePtr inst,
 		      xsltStylePreCompPtr castedComp,
-                      int fromset) {
+                      int fromAttributeSet) {
 #ifdef XSLT_REFACTORED
     xsltStyleItemAttributePtr comp =
 	(xsltStyleItemAttributePtr) castedComp;
+    /*
+    * TODO: Change the fields of the compiled struct:
+    *  1) @name (char)
+    *  2) @nameType (String, AVT)
+    *  3) @nsName (char)
+    *  4) nsNameType (None, String, AVT)
+    */
 #else
     xsltStylePreCompPtr comp = castedComp;
 #endif
-    xmlChar *prop = NULL;
-    xmlChar *namespace;
-    const xmlChar *name = NULL;
-    const xmlChar *prefix = NULL;
+    xmlNodePtr targetElem;
+    xmlChar *prop = NULL;    
+    const xmlChar *name = NULL, *prefix = NULL, *nsName = NULL;
     xmlChar *value = NULL;
     xmlNsPtr ns = NULL;
-    xmlAttrPtr attr;
-    const xmlChar *URL = NULL;
+    xmlAttrPtr attr;    
 
+    if ((ctxt == NULL) || (node == NULL) || (inst == NULL))
+        return;
+    /*
+    * BIG NOTE: This previously used xsltGetSpecialNamespace() and
+    *  xsltGetNamespace(), but since both are not appropriate, we
+    *  will process namespace lookup here to avoid adding yet another
+    *  ns-lookup function to namespaces.c.
+    */
+    /*
+    * SPEC XSLT 1.0: Error cases:
+    * - Creating nodes other than text nodes during the instantiation of
+    *   the content of the xsl:attribute element; implementations may
+    *   either signal the error or ignore the offending nodes."
+    */
 
-    if ((comp == NULL) || (ctxt == NULL)) {
+    if (comp == NULL) {
         xsltTransformError(ctxt, NULL, inst,
-                         "xsl:attribute : compilation failed\n");
+	    "Internal error in xsltAttributeInternal(): "
+	    "The instruction was no compiled.\n");
         return;
-    }
+    }    
+    /*
+    * TODO: Shouldn't ctxt->insert == NULL be treated as an internal error?
+    *   So report an internal error?
+    */
     if (ctxt->insert == NULL)
-        return;
-
-    if ((node == NULL) || (inst == NULL) || (comp == NULL))
-        return;
-    if (!comp->has_name) {
-        return;
-    }
-    if (ctxt->insert->children != NULL) {
+        return;    
+    /*
+    * SPEC XSLT 1.0:
+    *  "Adding an attribute to a node that is not an element;
+    *  implementations may either signal the error or ignore the attribute."
+    *
+    * TODO: I think we should signal such errors in the future, and maybe
+    *  provide an option to ignore such errors.
+    */
+    targetElem = ctxt->insert;
+    if (targetElem->type != XML_ELEMENT_NODE)
+	return;
+    
+    /*
+    * SPEC XSLT 1.0:
+    * "Adding an attribute to an element after children have been added
+    *  to it; implementations may either signal the error or ignore the
+    *  attribute."
+    *
+    * TODO: We should decide whether not to report such errors or
+    *  to ignore them; note that we *ignore* if the parent is not an
+    *  element, but here we report an error.
+    */
+    if (targetElem->children != NULL) {
+	/*
+	* NOTE: Ah! This seems to be intended to support streamed
+	*  result generation!.
+	*/
         xsltTransformError(ctxt, NULL, inst,
-                         "xsl:attribute : node already has children\n");
+	    "xsl:attribute: Cannot add attributes to an "
+	    "element if children have been already added "
+	    "to the element.\n");
         return;
     }
+
+    /*
+    * Process the name
+    * ----------------
+    */
+    if (!comp->has_name) /* TODO: raise error */
+        return;
+
 #ifdef WITH_DEBUGGER
-    if (ctxt->debugStatus != XSLT_DEBUG_NONE) {
+    if (ctxt->debugStatus != XSLT_DEBUG_NONE)
         xslHandleDebugger(inst, node, NULL, ctxt);
-    }
 #endif
 
     if (comp->name == NULL) {
-        prop =
-            xsltEvalAttrValueTemplate(ctxt, inst, (const xmlChar *) "name",
-                                      XSLT_NAMESPACE);
+	/* TODO: fix attr acquisition wrt to the XSLT namespace */
+        prop = xsltEvalAttrValueTemplate(ctxt, inst,
+	    (const xmlChar *) "name", XSLT_NAMESPACE);
         if (prop == NULL) {
             xsltTransformError(ctxt, NULL, inst,
-                             "xsl:attribute : name is missing\n");
+		"xsl:attribute: The attribute 'name' is missing.\n");
             goto error;
         }
 	if (xmlValidateQName(prop, 0)) {
 	    xsltTransformError(ctxt, NULL, inst,
-			    "xsl:attribute : invalid QName\n");
+		"xsl:attribute: The effective name '%s' is not a "
+		"valid QName.\n", prop);
 	    /* we fall through to catch any further errors, if possible */
 	}
 	name = xsltSplitQName(ctxt->dict, prop, &prefix);
@@ -704,59 +758,211 @@ xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
     if (!xmlStrncasecmp(prefix, (xmlChar *) "xmlns", 5)) {
 #ifdef WITH_XSLT_DEBUG_PARSING
         xsltGenericDebug(xsltGenericDebugContext,
-                         "xsltAttribute: xmlns prefix forbidden\n");
+	    "xsltAttribute: xmlns prefix forbidden\n");
 #endif
+	/*
+	* SPEC XSLT 1.0:
+	*  "It is an error if the string that results from instantiating
+	*  the attribute value template is not a QName or is the string
+	*  xmlns. An XSLT processor may signal the error; if it does not
+	*  signal the error, it must recover by not adding the attribute
+	*  to the result tree."
+	* TODO: Decide which way to go here.
+	*/
         goto error;
     }
-    if ((comp->ns == NULL) && (comp->has_ns)) {
-        namespace = xsltEvalAttrValueTemplate(ctxt, inst,
-                                              (const xmlChar *)
-                                              "namespace", XSLT_NAMESPACE);
+    /*
+    * Process namespace semantics
+    * ---------------------------
+    *
+    * Evaluate the namespace name.
+    */
+    if (comp->has_ns) {	 
+	if (comp->ns != NULL) {
+	    if (comp->ns[0] != 0)
+		nsName = comp->ns;
+	} else {
+	    xmlChar *tmpNsName;
+	    /*
+	    * Eval the AVT.
+	    */
+	    /* TODO: check attr acquisition wrt to the XSLT namespace */
+	    tmpNsName = xsltEvalAttrValueTemplate(ctxt, inst,
+		(const xmlChar *) "namespace", XSLT_NAMESPACE);	
+	    /*
+	    * This fixes bug #302020: The AVT might also evaluate to the 
+	    * empty string; this means that the empty string also indicates
+	    * "no namespace".
+	    * SPEC XSLT 1.0:
+	    *  "If the string is empty, then the expanded-name of the
+	    *  attribute has a null namespace URI."
+	    */
+	    if ((tmpNsName != NULL) && (tmpNsName[0] != 0))
+		nsName = xmlDictLookup(ctxt->dict, BAD_CAST tmpNsName, -1);
+	    xmlFree(tmpNsName);		
+	};	    
+    } else if (prefix != NULL) {
+	/*
+	* SPEC XSLT 1.0:
+	*  "If the namespace attribute is not present, then the QName is
+	*  expanded into an expanded-name using the namespace declarations
+	*  in effect for the xsl:attribute element, *not* including any
+	*  default namespace declaration."
+	*/	
+	ns = xmlSearchNs(inst->doc, inst, prefix);
+	if (ns == NULL) {
+	    /*
+	    * Note that this is treated as an error now (checked with
+	    *  Saxon, Xalan-J and MSXML).
+	    */
+	    xsltTransformError(ctxt, NULL, inst,
+		"xsl:attribute: The effective prefix '%s', has no "
+		"namespace binding in scope in the stylesheet; "
+		"this is an error, since the namespace was not "
+		"specified by the instruction itself.\n", prefix);
+	} else
+	    nsName = ns->href;	
+    }
+
+    if (fromAttributeSet) {
 	/*
-	* This fixes bug #302020: check also for the empty string.
+	* I think 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).
 	*/
-        if ((namespace != NULL) && (*namespace != 0)) {
-            ns = xsltGetSpecialNamespace(ctxt, inst, namespace, prefix,
-                                         ctxt->insert);
-            xmlFree(namespace);
-        } else {
-            if (prefix != NULL) {
-                ns = xmlSearchNs(inst->doc, inst, prefix);
-                if (ns == NULL) {
-                    xsltTransformError(ctxt, NULL, inst,
-			 "xsl:attribute : no namespace bound to prefix %s\n",
-                                     prefix);
-                } else {
-                    ns = xsltGetNamespace(ctxt, inst, ns, ctxt->insert);
-                }
-            }
-        }
-    } else if (comp->ns != NULL) {
-        ns = xsltGetSpecialNamespace(ctxt, inst, comp->ns, prefix,
-                                     ctxt->insert);
-    } else if (prefix != NULL) {
-	xmlNsPtr tmp;
-	tmp = xmlSearchNs(inst->doc, inst, prefix);
-	if (tmp != NULL) {
-	    ns = xsltGetNamespace(ctxt, inst, tmp, ctxt->insert);
+	attr = xmlHasNsProp(targetElem, name, nsName);
+	if (attr != NULL)
+	    return;
+    }
+
+    /*
+    * Something about ns-prefixes:
+    * SPEC XSLT 1.0:
+    *  "XSLT processors may make use of the prefix of the QName specified
+    *  in the name attribute when selecting the prefix used for outputting
+    *  the created attribute as XML; however, they are not required to do
+    *  so and, if the prefix is xmlns, they must not do so"
+    */        
+    /*
+    * Find/create a matching ns-decl in the result tree.
+    */
+    ns = NULL;
+#if 0
+    if (0) {	
+	/*
+	* OPTIMIZE TODO: How do we know if we are adding to a
+	*  fragment or not?
+	*
+	* If we are adding to a result tree fragment (i.e., not to the
+	* actual result tree), we'll don't bother searching for the
+	* ns-decl, but just store it in the dummy-doc of the result
+	* tree fragment.
+	*/
+	if (nsName != NULL) {
+	    ns = xsltTreeAcquireStoredNs(ctxt->document->doc, nsName,
+		prefix);
 	}
     }
+#endif
+    if (nsName != NULL) {
+	/*
+	* The owner-element might be in the same namespace.
+	*/
+	if ((targetElem->ns != NULL) &&
+	    (targetElem->ns->prefix != NULL) &&	    
+	    xmlStrEqual(targetElem->ns->href, nsName))
+	{
+	    ns = targetElem->ns;
+	    goto namespace_finished;
+	}
+	if (prefix != NULL) {
+	    /*
+	    * Search by ns-prefix.
+	    */
+	    ns = xmlSearchNs(targetElem->doc, targetElem, prefix);
+	    if ((ns != NULL) && (xmlStrEqual(ns->href, nsName))) {
+		goto namespace_finished;
+	    }
+	}
+	/*
+	* Fallback to a search by ns-name.
+	*/	
+	ns = xmlSearchNsByHref(targetElem->doc, targetElem, nsName);
+	if ((ns != NULL) && (ns->prefix != NULL)) {
+	    goto namespace_finished;
+	}
+	/*
+	* OK, we need to declare the namespace on the target element.
+	*/
+	if (prefix) {
+	    if (targetElem->nsDef != NULL) {
+		ns = targetElem->nsDef;
+		do {
+		    if ((ns->prefix) && xmlStrEqual(ns->prefix, prefix)) {
+			/*
+			* The prefix is aready occupied.
+			*/
+			break;
+		    }
+		    ns = ns->next;
+		} while (ns != NULL);
+		if (ns == NULL) {
+		    ns = xmlNewNs(targetElem, nsName, prefix);
+		    goto namespace_finished;
+		}
+	    }
+	}
+	/*
+	* Generate a new prefix.
+	*/
+	{
+	    const xmlChar *basepref = prefix;
+	    xmlChar pref[30];
+	    int counter = 0;
+	    
+	    if (prefix != NULL)
+		basepref = prefix;
+	    else
+		basepref = xmlStrdup(BAD_CAST "ns");
+	    
+	    do {
+		snprintf((char *) pref, 30, "%s_%d",
+		    basepref, counter++);
+		ns = xmlSearchNs(targetElem->doc, targetElem, BAD_CAST pref);
+		if (counter > 1000) {
+		    xsltTransformError(ctxt, NULL, inst,
+		    	"Namespace fixup error: Failed to compute a "
+			"new unique ns-prefix for the generated attribute "
+			"{%s}%s'.\n", nsName, name);		    			
+		    ns = NULL;
+		    break;
+		}
+	    } while (ns != NULL);
+	    if (basepref != prefix)
+		xmlFree((xmlChar *)basepref);
+	    ns = xmlNewNs(targetElem, nsName, BAD_CAST pref);
+	}
 
-    if ((fromset) && (ns != NULL))
-        URL = ns->href;
+namespace_finished:
 
-    if (fromset) {
-	if (URL != NULL)
-	    attr = xmlHasNsProp(ctxt->insert, name, URL);
-	else
-	    attr = xmlHasProp(ctxt->insert, name);
-	if (attr != NULL)
-	    return;
+	if (ns == NULL) {
+	    xsltTransformError(ctxt, NULL, inst,
+		"Namespace fixup error: Failed to acquire an in-scope "
+		"namespace binding of the generated attribute '{%s}%s'.\n",
+		nsName, name);
+	    /*
+	    * TODO: Should we just stop here?
+	    */
+	}
     }
-    
+    /*
+    * Construction of the value
+    * -------------------------
+    */
     if (inst->children == NULL) {
 	/*
 	* No content.
+	* TODO: Do we need to put the empty string in ?
 	*/
 	attr = xmlSetNsProp(ctxt->insert, ns, name, (const xmlChar *) "");
     } else if ((inst->children->next == NULL) && 
@@ -768,11 +974,10 @@ xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	* Optimization: if the content is just 1 text node, then
 	* just need to copy it over, or just assign it to the result
 	* if the string is shared.
-	* NOTE that xmlSetNsProp() will query if this is an ID attribute :-/
 	*/
 	attr = xmlSetNsProp(ctxt->insert, ns, name, NULL);
-	if (attr == NULL) /* TODO: report error */
-	    goto internal_err;
+	if (attr == NULL) /* TODO: report error ? */
+	    goto error;
 	/*
 	* This was taken over from xsltCopyText() (transform.c).
 	*/
@@ -782,7 +987,7 @@ xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	{
 	    copyTxt = xmlNewText(NULL);
 	    if (copyTxt == NULL) /* TODO: report error */
-		goto internal_err;
+		goto error;
 	    /*
 	    * This is a safe scenario where we don't need to lookup
 	    * the dict.
@@ -790,22 +995,29 @@ xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	    copyTxt->content = origTxt->content;
 	    /*
 	    * Copy "disable-output-escaping" information.
+	    * TODO: Does this have any effect for attribute values
+	    *  anyway?
 	    */
 	    if (origTxt->name == xmlStringTextNoenc)
-		copyTxt->name = xmlStringTextNoenc;				
+		copyTxt->name = xmlStringTextNoenc;
 	} else {
 	    /*
 	    * Copy the value.
 	    */
 	    copyTxt = xmlNewText(origTxt->content);
 	    if (copyTxt == NULL) /* TODO: report error */
-		goto internal_err;
+		goto error;
+	    /*
+	    * Copy "disable-output-escaping" information.
+	    * TODO: Does this have any effect for attribute values
+	    *  anyway?
+	    */
 	    if (origTxt->name == xmlStringTextNoenc)
-		copyTxt->name = xmlStringTextNoenc;		
+		copyTxt->name = xmlStringTextNoenc;
 	}
-	if (copyTxt != NULL) {	    
+	if (copyTxt != NULL) {
 	    copyTxt->doc = attr->doc;
-	    xmlAddChild((xmlNodePtr) attr, copyTxt);	    
+	    xmlAddChild((xmlNodePtr) attr, copyTxt);
 	}
     } else {
 	/*
@@ -820,13 +1032,11 @@ xsltAttributeInternal(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	    * TODO: Do we have to add the empty string to the attr?
 	    */
 	    attr = xmlSetNsProp(ctxt->insert, ns, name,
-		(const xmlChar *) "");	    
-	    
+		(const xmlChar *) "");
 	}
     }
 
 error:
-internal_err:
     return;    
 }
 
diff --git a/libxslt/transform.c b/libxslt/transform.c
index 88df88f..ed4d4db 100644
--- a/libxslt/transform.c
+++ b/libxslt/transform.c
@@ -366,8 +366,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
     if (cur->templTab == NULL) {
 	xsltTransformError(NULL, NULL, (xmlNodePtr) doc,
 		"xsltNewTransformContext: out of memory\n");
-	xmlFree(cur);
-	return(NULL);
+	goto internal_err;
     }
     cur->templNr = 0;
     cur->templMax = 5;
@@ -381,9 +380,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
     if (cur->varsTab == NULL) {
         xmlGenericError(xmlGenericErrorContext,
 		"xsltNewTransformContext: out of memory\n");
-	xmlFree(cur->templTab);
-	xmlFree(cur);
-	return(NULL);
+	goto internal_err;
     }
     cur->varsNr = 0;
     cur->varsMax = 5;
@@ -404,14 +401,15 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
     if (cur->xpathCtxt == NULL) {
 	xsltTransformError(NULL, NULL, (xmlNodePtr) doc,
 		"xsltNewTransformContext : xmlXPathNewContext failed\n");
-	xmlFree(cur->templTab);
-	xmlFree(cur->varsTab);
-	xmlFree(cur);
-	return(NULL);
+	goto internal_err;
     }
     cur->xpathCtxt->proximityPosition = 0;
     cur->xpathCtxt->contextSize = 0;
-
+    /*
+    * Create an XPath cache.
+    */
+    if (xmlXPathContextSetCache(cur->xpathCtxt, 1, -1, 0) == -1)
+	goto internal_err;
     /*
      * Initialize the extras array
      */
@@ -422,11 +420,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
 	if (cur->extras == NULL) {
 	    xmlGenericError(xmlGenericErrorContext,
 		    "xsltNewTransformContext: out of memory\n");
-	    xmlFree(cur->xpathCtxt);
-	    xmlFree(cur->varsTab);
-	    xmlFree(cur->templTab);
-	    xmlFree(cur);
-	    return(NULL);
+	    goto internal_err;
 	}
 	cur->extrasNr = style->extrasNr;
 	for (i = 0;i < cur->extrasMax;i++) {
@@ -462,10 +456,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
     if (docu == NULL) {
 	xsltTransformError(cur, NULL, (xmlNodePtr)doc,
 		"xsltNewTransformContext : xsltNewDocument failed\n");
-	xmlFree(cur->templTab);
-	xmlFree(cur->varsTab);
-	xmlFree(cur);
-	return(NULL);
+	goto internal_err;
     }
     docu->main = 1;
     cur->document = docu;
@@ -477,6 +468,11 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
     cur->xinclude = xsltGetXIncludeDefault();
 
     return(cur);
+
+internal_err:
+    if (cur != NULL)
+	xsltFreeTransformContext(cur);
+    return(NULL);
 }
 
 /**
@@ -778,6 +774,11 @@ xsltCopyText(xsltTransformContextPtr ctxt, xmlNodePtr target,
     if (copy != NULL) {
 	if (target != NULL) {
 	    copy->doc = target->doc;
+	    /*
+	    * MAYBE TODO: Maybe we should reset the ctxt->lasttext here
+	    *  to ensure that the optimized text-merging mechanism
+	    *  won't interfere with normal node-merging in any case.
+	    */
 	    xmlAddChild(target, copy);
 	}
     } else {
@@ -790,35 +791,190 @@ xsltCopyText(xsltTransformContextPtr ctxt, xmlNodePtr target,
 /**
  * xsltCopyProp:
  * @ctxt:  a XSLT process context
- * @target:  the element where the attribute will be grafted
- * @attr:  the attribute
+ * @targetElem:  the element where the attribute will be grafted
+ * @attr: the attribute to be copied
  *
  * Do a copy of an attribute
  *
  * Returns: a new xmlAttrPtr, or NULL in case of error.
  */
 static xmlAttrPtr
-xsltCopyProp(xsltTransformContextPtr ctxt, xmlNodePtr target,
-	     xmlAttrPtr attr) {
-    xmlAttrPtr ret = NULL;
-    xmlNsPtr ns;
-    xmlChar *val;
+xsltCopyProp(xsltTransformContextPtr ctxt, xmlNodePtr targetElem,
+	     xmlAttrPtr attr)
+{
+    xmlAttrPtr attrCopy;
+    xmlChar *value;
+#ifdef XSLT_REFACTORED 
+    xmlNodePtr txtNode;
+#endif
 
     if (attr == NULL)
 	return(NULL);
-    if (target->type != XML_ELEMENT_NODE)
+
+    if (targetElem->type != XML_ELEMENT_NODE) {
+	/*
+	* TODO: Hmm, it would be better to have the node at hand of the
+	*  instruction which lead to this here.
+	*/
+	xsltTransformError(ctxt, NULL, NULL,
+	    "Result tree construction error: cannot set an attribute node "
+	    "on a non-element node.\n");
 	return(NULL);
+    }    
 
+#ifdef XSLT_REFACTORED
+    /*
+    * Create the attribute node.
+    */
     if (attr->ns != NULL) {
-	ns = xsltGetPlainNamespace(ctxt, attr->parent, attr->ns, target);
+	xmlNsPtr ns = NULL;
+	const xmlChar *prefix = attr->ns->prefix;
+	
+	/*
+	* Process namespace semantics
+	*
+	* RESTRUCTURE TODO: This is the same code as in
+	*  xsltAttributeInternal() (attributes.c), but I currently
+	*  don't want to add yet another ns-lookup function.
+	*/
+	if ((targetElem->ns != NULL) &&
+	    (targetElem->ns->prefix != NULL) &&
+	    xmlStrEqual(targetElem->ns->href, attr->ns->href))
+	{
+	    ns = targetElem->ns;
+	    goto namespace_finished;
+	}
+	if (prefix != NULL) {
+	    /*
+	    * Search by ns-prefix.
+	    */
+	    ns = xmlSearchNs(targetElem->doc, targetElem, prefix);
+	    if ((ns != NULL) &&
+		(xmlStrEqual(ns->href, attr->ns->href)))
+	    {
+		goto namespace_finished;
+	    }
+	}
+	/*
+	* Fallback to a search by ns-name.
+	*/	
+	ns = xmlSearchNsByHref(targetElem->doc, targetElem, attr->ns->href);
+	if ((ns != NULL) && (ns->prefix != NULL)) {
+	    goto namespace_finished;
+	}
+	/*
+	* OK, we need to declare the namespace on the target element.
+	*/
+	if (prefix) {
+	    if (targetElem->nsDef != NULL) {
+		ns = targetElem->nsDef;
+		do {
+		    if ((ns->prefix) && xmlStrEqual(ns->prefix, prefix)) {
+			/*
+			* The prefix aready occupied.
+			*/
+			break;
+		    }
+		    ns = ns->next;
+		} while (ns != NULL);
+		if (ns == NULL) {
+		    ns = xmlNewNs(targetElem, attr->ns->href, prefix);
+		    goto namespace_finished;
+		}
+	    }
+	}
+	/*
+	* Generate a new prefix.
+	*/
+	{
+	    const xmlChar *basepref = prefix;
+	    xmlChar pref[30];
+	    int counter = 0;
+	    
+	    if (prefix != NULL)
+		basepref = prefix;
+	    else
+		basepref = xmlStrdup(BAD_CAST "ns");
+	    
+	    do {
+		snprintf((char *) pref, 30, "%s_%d",
+		    basepref, counter++);
+		ns = xmlSearchNs(targetElem->doc,
+		    (xmlNodePtr) attr, BAD_CAST pref);
+		if (counter > 1000) {
+		    xsltTransformError(ctxt, NULL, (xmlNodePtr) attr,
+			"Namespace fixup error: Failed to compute a "
+			"new unique ns-prefix for the copied attribute "
+			"{%s}%s'.\n", attr->ns->href, attr->name);
+		    ns = NULL;
+		    break;
+		}
+	    } while (ns != NULL);
+	    if (basepref != prefix)
+		xmlFree((xmlChar *)basepref);
+	    ns = xmlNewNs(targetElem, attr->ns->href, BAD_CAST pref);
+	}
+
+namespace_finished:
+
+	if (ns == NULL) {
+	    xsltTransformError(ctxt, NULL, (xmlNodePtr) attr,
+		"Namespace fixup error: Failed to acquire an in-scope "
+		"namespace binding of the copied attribute '{%s}%s'.\n",
+		attr->ns->href, attr->name);
+	    /*
+	    * TODO: Should we just stop here?
+	    */
+	}
+	attrCopy = xmlSetNsProp(targetElem, ns, attr->name, NULL);
     } else {
-	ns = NULL;
+	attrCopy = xmlSetNsProp(targetElem, NULL, attr->name, NULL);
     }
-    val = xmlNodeListGetString(attr->doc, attr->children, 1);
-    ret = xmlSetNsProp(target, ns, attr->name, val);
-    if (val != NULL)
-	xmlFree(val);
-    return(ret);
+    if (attrCopy == NULL)	
+	return(NULL);    
+    /*
+    * NOTE: This was optimized according to bug #342695.
+    * TODO: Can this further be optimized, if source and target
+    *  share the same dict and attr->children is just 1 text node
+    *  which is in the dict? How probable is such a case?
+    */
+    value = xmlNodeListGetString(attr->doc, attr->children, 1);
+    if (value != NULL) {
+	txtNode = xmlNewDocText(targetElem->doc, NULL);
+	if (txtNode == NULL)
+	    return(NULL);
+	if ((targetElem->doc != NULL) &&
+	    (targetElem->doc->dict != NULL))
+	{
+	    txtNode->content =
+		(xmlChar *) xmlDictLookup(targetElem->doc->dict,
+		    BAD_CAST value, -1);
+	    xmlFree(value);	    
+	} else
+	    txtNode->content = value;
+	attrCopy->children = txtNode;
+    }
+    /*
+    * URGENT TODO: Do we need to create an empty text node if the value
+    *  is the empty string?
+    */    
+
+#else /* not XSLT_REFACTORED */
+
+    value = xmlNodeListGetString(attr->doc, attr->children, 1);
+    if (attr->ns != NULL) {
+	xmlNsPtr ns;
+	ns = xsltGetPlainNamespace(ctxt, attr->parent, attr->ns, targetElem);
+	attrCopy = xmlSetNsProp(targetElem, ns, attr->name, value);
+    } else {
+	attrCopy = xmlSetNsProp(targetElem, NULL, attr->name, value);
+    }
+    if (value != NULL)
+	xmlFree(value);
+
+#endif /* not XSLT_REFACTORED */
+
+    return(attrCopy);
 }
 
 /**
@@ -1015,7 +1171,8 @@ xsltCopyNamespaceListInternal(xmlNodePtr node, xmlNsPtr cur) {
  * Make a copy of the full tree under the element node @node
  * and insert it as last child of @insert
  * For literal result element, some of the namespaces may not be copied
- * over according to section 7.1 .
+ * over according to section 7.1.
+ * TODO: Why is this a public function?
  *
  * Returns a pointer to the new tree, or NULL in case of error
  */
@@ -1089,13 +1246,11 @@ xsltCopyTree(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	    (node->type == XML_ATTRIBUTE_NODE)) {
 	    xmlNsPtr *nsList, *cur, ns;
 	    /*
-	     * must add in any new namespaces in scope for the node
-	     * REVISIT:
-	     *   Question: Do we really have to add every namespace in scope?
-	     *   Answer: I think yes, since if we are only adding the
-	     *     ns-decls which are declared on this element and actually
-	     *     referenced by this element/attribute, then we would miss
-	     *     ns-decls for QName in element/attribute content.
+	     * Must add any new namespaces in scope for the node.
+	     * TODO: Since we try to reuse existing in-scope ns-decls by
+	     *  using xmlSearchNsByHref(), this will eventually change
+	     *  the prefix of an original ns-binding; thus it might
+	     *  break QNames in element/attribute content.
 	     */
 	    nsList = xmlGetNsList(node->doc, node);
 	    if (nsList != NULL) {
@@ -1108,9 +1263,15 @@ xsltCopyTree(xsltTransformContextPtr ctxt, xmlNodePtr node,
 		}
 		xmlFree(nsList);
 	    }
-	    if (node->ns != NULL)
-		copy->ns = xsltGetNamespace(ctxt, node, node->ns, copy);
-	    else if ((insert->type == XML_ELEMENT_NODE) && (insert->ns != NULL)) {
+	    if (node->ns != NULL) {
+		/*
+		* This will map  copy->ns to one of the newly created
+		* in-scope ns-decls.
+		*/
+		copy->ns = xsltGetNamespace(ctxt, node, node->ns, copy);		
+	    } else if ((insert->type == XML_ELEMENT_NODE) &&
+		(insert->ns != NULL))
+	    {
 		xmlNsPtr defaultNs;
 
 		defaultNs = xmlSearchNs(insert->doc, insert, NULL);
@@ -1588,10 +1749,10 @@ xsltTransLREUndeclareResultDefaultNs(xsltTransformContextPtr ctxt,
 
 /**
 * xsltTransLREAcquireResultInScopeNs:
-* @ctxt:  the transformation context
-* @cur:  the literal result element
-* @ns:  the namespace
-* @out:  the output node (or its parent)
+* @ctxt: the transformation context
+* @cur: the literal result element (in the stylesheet)
+* @literalNs: the namespace (in the stylsheet)
+* @resultElem: the generated result element
 *
 *
 * Find a matching (prefix and ns-name) ns-declaration
@@ -1606,7 +1767,8 @@ xsltTransLREUndeclareResultDefaultNs(xsltTransformContextPtr ctxt,
 */
 static xmlNsPtr
 xsltTransLREAcquireResultInScopeNs(xsltTransformContextPtr ctxt,
-				   xmlNodePtr cur, xmlNsPtr literalNs,
+				   xmlNodePtr cur,
+				   xmlNsPtr literalNs,
 				   xmlNodePtr resultElem)
 {    
     xmlNsPtr ns;
@@ -3188,7 +3350,7 @@ xsltCopy(xsltTransformContextPtr ctxt, xmlNodePtr node,
     xsltStylePreCompPtr comp = castedComp;
 #endif
     xmlNodePtr copy, oldInsert;
-
+   
     oldInsert = ctxt->insert;
     if (ctxt->insert != NULL) {
 	switch (node->type) {
@@ -3631,12 +3793,12 @@ error:
 
 /**
  * xsltCopyOf:
- * @ctxt:  a XSLT process context
- * @node:  the node in the source tree.
- * @inst:  the xslt copy-of node
- * @comp:  precomputed information
+ * @ctxt:  an XSLT transformation context
+ * @node:  the current node in the source tree
+ * @inst:  the element node of the XSLT copy-of instruction 
+ * @comp:  precomputed information of the XSLT copy-of instruction
  *
- * Process the xslt copy-of node on the source node
+ * Process the XSLT copy-of instruction.
  */
 void
 xsltCopyOf(xsltTransformContextPtr ctxt, xmlNodePtr node,
@@ -3661,11 +3823,32 @@ xsltCopyOf(xsltTransformContextPtr ctxt, xmlNodePtr node,
 	return;
     }
 
+     /*
+    * SPEC XSLT 1.0:
+    *  "The xsl:copy-of element can be used to insert a result tree
+    *  fragment into the result tree, without first converting it to
+    *  a string as xsl:value-of does (see [7.6.1 Generating Text with
+    *  xsl:value-of]). The required select attribute contains an
+    *  expression. When the result of evaluating the expression is a
+    *  result tree fragment, the complete fragment is copied into the
+    *  result tree. When the result is a node-set, all the nodes in the
+    *  set are copied in document order into the result tree; copying
+    *  an element node copies the attribute nodes, namespace nodes and
+    *  children of the element node as well as the element node itself;
+    *  a root node is copied by copying its children. When the result
+    *  is neither a node-set nor a result tree fragment, the result is
+    *  converted to a string and then inserted into the result tree,
+    *  as with xsl:value-of.
+    */
+
 #ifdef WITH_XSLT_DEBUG_PROCESS
     XSLT_TRACE(ctxt,XSLT_TRACE_COPY_OF,xsltGenericDebug(xsltGenericDebugContext,
 	 "xsltCopyOf: select %s\n", comp->select));
 #endif
 
+    /*
+    * Set up the XPath evaluation context.
+    */
     oldProximityPosition = ctxt->xpathCtxt->proximityPosition;
     oldContextSize = ctxt->xpathCtxt->contextSize;
     oldNsNr = ctxt->xpathCtxt->nsNr;
@@ -3683,57 +3866,79 @@ xsltCopyOf(xsltTransformContextPtr ctxt, xmlNodePtr node,
     ctxt->xpathCtxt->namespaces = comp->nsList;
     ctxt->xpathCtxt->nsNr = comp->nsNr;
 #endif
+    /*
+    * Evaluate the "select" expression.
+    */
     res = xmlXPathCompiledEval(comp->comp, ctxt->xpathCtxt);
+    /*
+    * Revert the XPath evaluation context to previous state.
+    */
     ctxt->xpathCtxt->proximityPosition = oldProximityPosition;
     ctxt->xpathCtxt->contextSize = oldContextSize;
     ctxt->xpathCtxt->nsNr = oldNsNr;
     ctxt->xpathCtxt->namespaces = oldNamespaces;
+
     if (res != NULL) {
 	if (res->type == XPATH_NODESET) {
+	    /*
+	    * Node-set
+	    * --------
+	    */
 #ifdef WITH_XSLT_DEBUG_PROCESS
 	    XSLT_TRACE(ctxt,XSLT_TRACE_COPY_OF,xsltGenericDebug(xsltGenericDebugContext,
 		 "xsltCopyOf: result is a node set\n"));
 #endif
 	    list = res->nodesetval;
 	    if (list != NULL) {
-		/* the list is already sorted in document order by XPath */
-		/* append everything in this order under ctxt->insert */
+		xmlNodePtr cur;
+		/*
+		* The list is already sorted in document order by XPath.
+		* Append everything in this order under ctxt->insert.
+		*/
 		for (i = 0;i < list->nodeNr;i++) {
-		    if (list->nodeTab[i] == NULL)
+		    cur = list->nodeTab[i];
+		    if (cur == NULL)
 			continue;
-		    if ((list->nodeTab[i]->type == XML_DOCUMENT_NODE) ||
-			(list->nodeTab[i]->type == XML_HTML_DOCUMENT_NODE)) {
-			xsltCopyTreeList(ctxt, list->nodeTab[i]->children,
-				         ctxt->insert, 0);
-		    } else if (list->nodeTab[i]->type == XML_ATTRIBUTE_NODE) {
-			xsltCopyProp(ctxt, ctxt->insert, 
-				     (xmlAttrPtr) list->nodeTab[i]);
+		    if ((cur->type == XML_DOCUMENT_NODE) ||
+			(cur->type == XML_HTML_DOCUMENT_NODE))
+		    {
+			xsltCopyTreeList(ctxt, cur->children, ctxt->insert, 0);
+		    } else if (cur->type == XML_ATTRIBUTE_NODE) {
+			xsltCopyProp(ctxt, ctxt->insert, (xmlAttrPtr) cur);
 		    } else {
-			xsltCopyTree(ctxt, list->nodeTab[i], ctxt->insert, 0);
+			xsltCopyTree(ctxt, cur, ctxt->insert, 0);
 		    }
 		}
 	    }
 	} else if (res->type == XPATH_XSLT_TREE) {
+	    /*
+	    * Result tree fragment
+	    * --------------------
+	    */
 #ifdef WITH_XSLT_DEBUG_PROCESS
 	    XSLT_TRACE(ctxt,XSLT_TRACE_COPY_OF,xsltGenericDebug(xsltGenericDebugContext,
 		 "xsltCopyOf: result is a result tree fragment\n"));
 #endif
+	    /*
+	    * TODO: Is list->nodeTab[0] is an xmlDocPtr?
+	    */
 	    list = res->nodesetval;
 	    if ((list != NULL) && (list->nodeTab != NULL) &&
 		(list->nodeTab[0] != NULL) &&
-		(IS_XSLT_REAL_NODE(list->nodeTab[0]))) {
+		(IS_XSLT_REAL_NODE(list->nodeTab[0])))
+	    {
 		xsltCopyTreeList(ctxt, list->nodeTab[0]->children,
 			         ctxt->insert, 0);
 	    }
 	} else {
-	    /* convert to a string */
+	    /* Convert to a string. */
 	    res = xmlXPathConvertString(res);
 	    if ((res != NULL) && (res->type == XPATH_STRING)) {
 #ifdef WITH_XSLT_DEBUG_PROCESS
 		XSLT_TRACE(ctxt,XSLT_TRACE_COPY_OF,xsltGenericDebug(xsltGenericDebugContext,
 		     "xsltCopyOf: result %s\n", res->stringval));
 #endif
-		/* append content as text node */
+		/* Append content as text node. */
 		xsltCopyTextString(ctxt, ctxt->insert, res->stringval, 0);
 	    }
 	}
@@ -5193,6 +5398,10 @@ xsltApplyStylesheetInternal(xsltStylesheetPtr style, xmlDocPtr doc,
             }
             if (tmp == root) {
                 ctxt->type = XSLT_OUTPUT_HTML;
+		/*
+		* REVISIT TODO: XML_HTML_DOCUMENT_NODE is set after the
+		*  transformation on the doc, but functions like
+		*/
                 res->type = XML_HTML_DOCUMENT_NODE;
                 if (((doctypePublic != NULL) || (doctypeSystem != NULL))) {
                     res->intSubset = xmlCreateIntSubset(res, doctype,
diff --git a/libxslt/variables.c b/libxslt/variables.c
index a1fd3f8..93635b4 100644
--- a/libxslt/variables.c
+++ b/libxslt/variables.c
@@ -42,14 +42,15 @@ const xmlChar *xsltDocFragFake = (const xmlChar *) " fake node libxslt";
 
 /************************************************************************
  *									*
- *			Result Value Tree interfaces			*
+ *  Result Value Tree (Result Tree Fragment) interfaces			*
  *									*
  ************************************************************************/
 /**
  * xsltCreateRVT:
  * @ctxt:  an XSLT transformation context
  *
- * Create a result value tree
+ * Create a Result Value Tree
+ * (the XSLT 1.0 term for this is "Result Tree Fragment") 
  *
  * Returns the result value tree or NULL in case of error
  */
@@ -59,8 +60,9 @@ xsltCreateRVT(xsltTransformContextPtr ctxt)
     xmlDocPtr container;
 
     /*
-    * TODO: Couldn't we use an XML_DOCUMENT_FRAG_NODE for this?
-    */
+    * Question: Why is this function public?
+    * Answer: It is called by the EXSLT module.
+    */    
     if (ctxt == NULL) return(NULL);
 
     container = xmlNewDoc(NULL);
@@ -82,9 +84,10 @@ xsltCreateRVT(xsltTransformContextPtr ctxt)
 /**
  * xsltRegisterTmpRVT:
  * @ctxt:  an XSLT transformation context
- * @RVT:  a result value tree
+ * @RVT:  a result value tree (Result Tree Fragment)
  *
- * Register the result value tree for destruction at the end of the context
+ * Register the result value tree (XSLT 1.0 term: Result Tree Fragment)
+ * for destruction at the end of the context
  *
  * Returns 0 in case of success and -1 in case of error.
  */
@@ -103,9 +106,10 @@ xsltRegisterTmpRVT(xsltTransformContextPtr ctxt, xmlDocPtr RVT)
 /**
  * xsltRegisterPersistRVT:
  * @ctxt:  an XSLT transformation context
- * @RVT:  a result value tree
+ * @RVT:  a result value tree (Result Tree Fragment)
  *
- * Register the result value tree for destruction at the end of the processing
+ * Register the result value tree (XSLT 1.0 term: Result Tree Fragment)
+ * for destruction at the end of the processing
  *
  * Returns 0 in case of success and -1 in case of error.
  */
@@ -125,7 +129,8 @@ xsltRegisterPersistRVT(xsltTransformContextPtr ctxt, xmlDocPtr RVT)
  * xsltFreeRVTs:
  * @ctxt:  an XSLT transformation context
  *
- * Free all the registered result value tree of the transformation
+ * Free all the registered result value tree (Result Tree Fragment)
+ * of the transformation
  */
 void
 xsltFreeRVTs(xsltTransformContextPtr ctxt)
@@ -613,6 +618,14 @@ xsltEvalGlobalVariable(xsltStackElemPtr elem, xsltTransformContextPtr ctxt)
 #else
     precomp = elem->comp;
 #endif
+
+    /*
+    * OPTIMIZE TODO: We should consider if instantiating global vars/params
+    *  on a on-demand basis would be better. The vars/params don't
+    *  need to be evaluated if never called; and in the case of
+    *  global params, if values for such params are provided by the
+    *  user.
+    */
     if (elem->select != NULL) {
 	xmlXPathCompExprPtr comp = NULL;
 
diff --git a/libxslt/xslt.c b/libxslt/xslt.c
index ab73975..a643d0a 100644
--- a/libxslt/xslt.c
+++ b/libxslt/xslt.c
@@ -1763,7 +1763,10 @@ xsltTreeAcquireStoredNs(xmlDocPtr doc,
 
     if (doc == NULL)
 	return (NULL);
-    ns = xsltTreeEnsureXMLDecl(doc);
+    if (doc->oldNs != NULL)
+	ns = doc->oldNs;
+    else
+	ns = xsltTreeEnsureXMLDecl(doc);
     if (ns == NULL)
 	return (NULL);
     if (ns->next != NULL) {