valid: Restore ID lookup

Revert a change from d025cfb and don't overwrite ID table entries, so
that the first attribute will be returned if there are duplicate IDs.

This requires two other changes:

- Attributes in entity content are never added to the ID table. This
  seems reasonable.

- Remove the optimization to skip ID lookup when copying and the target
  document has an empty ID table. This also seems more correct since the
  document could have ID declarations nevertheless or we could be
  copying xml:ids into the document for the first time.

Fixes #757.

Author: nwellnhof

SAX2.c

@@ -1255,6 +1255,7 @@ xmlSAX2AttributeInternal(void *ctx, const xmlChar *fullname,
     } else
 #endif /* LIBXML_VALID_ENABLED */
            if (((ctxt->loadsubset & XML_SKIP_IDS) == 0) &&
+               (ctxt->input->entity == NULL) &&
                /* Don't create IDs containing entity references */
                (ret->children != NULL) &&
                (ret->children->type == XML_TEXT_NODE) &&
@@ -1987,6 +1988,7 @@ xmlSAX2AttributeNs(xmlParserCtxtPtr ctxt,
     } else
 #endif /* LIBXML_VALID_ENABLED */
            if (((ctxt->loadsubset & XML_SKIP_IDS) == 0) &&
+               (ctxt->input->entity == NULL) &&
                /* Don't create IDs containing entity references */
                (ret->children != NULL) &&
                (ret->children->type == XML_TEXT_NODE) &&

result/xmlid/id_tst4.xml

@@ -1,6 +1,6 @@
 Object is a Node Set :
 Set contains 1 nodes:
-1  ELEMENT err
+1  ELEMENT foo
     ATTRIBUTE id
       TEXT
         content=bar

tree.c

@@ -4059,7 +4059,6 @@ xmlCopyPropInternal(xmlDocPtr doc, xmlNodePtr target, xmlAttrPtr cur) {
      */
     if ((target != NULL) && (cur != NULL) &&
 	(target->doc != NULL) && (cur->doc != NULL) &&
-	(cur->doc->ids != NULL) &&
         (cur->parent != NULL) &&
         (cur->children != NULL)) {
         int res = xmlIsID(cur->doc, cur->parent, cur);

valid.c

@@ -2336,9 +2336,6 @@ xmlAddIDInternal(xmlAttrPtr attr, const xmlChar *value, xmlIDPtr *idPtr) {
     if (doc == NULL)
         return(0);
 
-    if (attr->id != NULL)
-        xmlRemoveID(doc, attr);
-
     /*
      * Create the ID table if needed.
      */
@@ -2349,14 +2346,8 @@ xmlAddIDInternal(xmlAttrPtr attr, const xmlChar *value, xmlIDPtr *idPtr) {
             return(-1);
     } else {
         id = xmlHashLookup(table, value);
-        if (id != NULL) {
-            if (id->attr != NULL) {
-                id->attr->id = NULL;
-                id->attr->atype = 0;
-            }
-            ret = 0;
-            goto done;
-        }
+        if (id != NULL)
+            return(0);
     }
 
     id = (xmlIDPtr) xmlMalloc(sizeof(xmlID));
@@ -2374,6 +2365,9 @@ xmlAddIDInternal(xmlAttrPtr attr, const xmlChar *value, xmlIDPtr *idPtr) {
         return(-1);
     }
 
+    if (attr->id != NULL)
+        xmlRemoveID(doc, attr);
+
     if (xmlHashAddEntry(table, value, id) < 0) {
 	xmlFreeID(id);
 	return(-1);
@@ -2383,7 +2377,6 @@ xmlAddIDInternal(xmlAttrPtr attr, const xmlChar *value, xmlIDPtr *idPtr) {
     if (idPtr != NULL)
         *idPtr = id;
 
-done:
     id->attr = attr;
     id->lineno = xmlGetLineNo(attr->parent);
     attr->atype = XML_ATTRIBUTE_ID;