Fix bug 329677. Sort service patch by Neil Rashbrook, r=ndeakin, sr=bzbarsky.

The rest of the patch by me, r=pike, sr=Neil Rashbrook.

content/xul/document/src/nsXULDocument.cpp

@@ -2159,7 +2159,10 @@ nsXULDocument::ApplyPersistentAttributes()
             continue;
 
         nsAutoString id;
-        nsXULContentUtils::MakeElementID(this, NS_ConvertASCIItoUTF16(uri), id);
+        nsXULContentUtils::MakeElementID(this, nsDependentCString(uri), id);
+
+        if (id.IsEmpty())
+            continue;
 
         // This will clear the array if there are no elements.
         GetElementsForID(id, elements);

content/xul/templates/src/nsXULContentUtils.cpp

@@ -233,6 +233,8 @@ nsXULContentUtils::GetElementResource(nsIContent* aElement, nsIRDFResource** aRe
     PRUnichar buf[128];
     nsFixedString id(buf, NS_ARRAY_LENGTH(buf), 0);
 
+    // Whoa.  Why the "id" attribute?  What if it's not even a XUL
+    // element?  This is totally bogus!
     aElement->GetAttr(kNameSpaceID_None, nsXULAtoms::id, id);
     if (id.IsEmpty())
         return NS_ERROR_FAILURE;
@@ -323,44 +325,27 @@ nsXULContentUtils::GetTextForNode(nsIRDFNode* aNode, nsAString& aResult)
 }
 
 nsresult
-nsXULContentUtils::MakeElementURI(nsIDocument* aDocument, const nsAString& aElementID, nsCString& aURI)
+nsXULContentUtils::MakeElementURI(nsIDocument* aDocument,
+                                  const nsAString& aElementID,
+                                  nsCString& aURI)
 {
     // Convert an element's ID to a URI that can be used to refer to
     // the element in the XUL graph.
 
-    if (aElementID.FindChar(':') > 0) {
-        // Assume it's absolute already. Use as is.
-        CopyUTF16toUTF8(aElementID, aURI);
-    }
-    else {
-        nsIURI *docURL = aDocument->GetDocumentURI();
+    nsIURI *docURL = aDocument->GetDocumentURI();
+    NS_ENSURE_TRUE(docURL, NS_ERROR_UNEXPECTED);
 
-        // XXX Urgh. This is so broken; I'd really just like to use
-        // NS_MakeAbsolueURI(). Unfortunatly, doing that breaks
-        // MakeElementID in some cases that I haven't yet been able to
-        // figure out.
-#define USE_BROKEN_RELATIVE_PARSING
-#ifdef USE_BROKEN_RELATIVE_PARSING
-        docURL->GetSpec(aURI);
+    nsCOMPtr<nsIURI> docURIClone;
+    nsresult rv = docURL->Clone(getter_AddRefs(docURIClone));
+    NS_ENSURE_SUCCESS(rv, rv);
 
-        if (aElementID.First() != '#') {
-            aURI.Append('#');
-        }
-        AppendUTF16toUTF8(aElementID, aURI);
-#else
-        nsXPIDLCString spec;
-        nsresult rv = NS_MakeAbsoluteURI(nsCAutoString(aElementID), docURL, getter_Copies(spec));
-        if (NS_SUCCEEDED(rv)) {
-            aURI = spec;
-        }
-        else {
-            NS_WARNING("MakeElementURI: NS_MakeAbsoluteURI failed");
-            aURI = aElementID;
-        }
-#endif
-    }
+    nsCOMPtr<nsIURL> mutableURL(do_QueryInterface(docURIClone));
+    NS_ENSURE_TRUE(mutableURL, NS_ERROR_NOT_AVAILABLE);
 
-    return NS_OK;
+    rv = mutableURL->SetRef(NS_ConvertUTF16toUTF8(aElementID));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return mutableURL->GetSpec(aURI);
 }
 
 
@@ -384,26 +369,24 @@ nsXULContentUtils::MakeElementResource(nsIDocument* aDocument, const nsAString&
 
 
 nsresult
-nsXULContentUtils::MakeElementID(nsIDocument* aDocument, const nsAString& aURI, nsAString& aElementID)
+nsXULContentUtils::MakeElementID(nsIDocument* aDocument,
+                                 const nsACString& aURI,
+                                 nsAString& aElementID)
 {
     // Convert a URI into an element ID that can be accessed from the
     // DOM APIs.
-    nsCAutoString spec;
-    aDocument->GetDocumentURI()->GetSpec(spec);
+    nsCOMPtr<nsIURI> uri;
+    nsresult rv = NS_NewURI(getter_AddRefs(uri), aURI,
+                            aDocument->GetDocumentCharacterSet().get());
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    // XXX FIX ME to not do a copy
-    nsAutoString str(aURI);
-    if (str.Find(spec.get()) == 0) {
-#ifdef USE_BROKEN_RELATIVE_PARSING
-        static const PRInt32 kFudge = 1;  // XXX assume '#'
-#else
-        static const PRInt32 kFudge = 0;
-#endif
-        PRInt32 len = spec.Length();
-        aElementID = Substring(aURI, len + kFudge, aURI.Length() - (len + kFudge));
-    }
-    else {
-        aElementID = aURI;
+    nsCOMPtr<nsIURL> url = do_QueryInterface(uri);
+    if (url) {
+        nsCAutoString ref;
+        url->GetRef(ref);
+        CopyUTF8toUTF16(ref, aElementID);
+    } else {
+        aElementID.Truncate();
     }
 
     return NS_OK;

content/xul/templates/src/nsXULContentUtils.h

@@ -98,14 +98,24 @@ public:
     static nsresult
     GetTextForNode(nsIRDFNode* aNode, nsAString& aResult);
 
+    /**
+     * Construct a URI from the element ID given.  This uses aElement as the
+     * ref and aDocument's document URI as the base.  If aDocument's document
+     * URI does not support refs, this will throw NS_ERROR_NOT_AVAILABLE.
+     */
     static nsresult
     MakeElementURI(nsIDocument* aDocument, const nsAString& aElementID, nsCString& aURI);
 
     static nsresult
     MakeElementResource(nsIDocument* aDocument, const nsAString& aElementID, nsIRDFResource** aResult);
 
+    /**
+     * Extract the element ID from aURI.  Note that aURI must be an absolute
+     * URI string in UTF8; the element ID is the ref from the URI.  If the
+     * scheme does not support refs, then the ID will be empty.
+     */
     static nsresult
-    MakeElementID(nsIDocument* aDocument, const nsAString& aURI, nsAString& aElementID);
+    MakeElementID(nsIDocument* aDocument, const nsACString& aURI, nsAString& aElementID);
 
     static nsresult
     GetResource(PRInt32 aNameSpaceID, nsIAtom* aAttribute, nsIRDFResource** aResult);

content/xul/templates/src/nsXULSortService.cpp

@@ -1099,7 +1099,8 @@ XULSortServiceImpl::SortContainer(nsIContent *container, sortPtr sortInfo,
         --currentElement;
 
         nsCOMPtr<nsIRDFResource>  resource;
-        nsXULContentUtils::GetElementResource(child, getter_AddRefs(resource));
+        nsCOMPtr<nsIDOMXULElement> xulElement(do_QueryInterface(child));
+        xulElement->GetResource(getter_AddRefs(resource));
         contentSortInfo *contentInfo = CreateContentSortInfo(child, resource);
         if (contentInfo)
           contentSortInfoArray[currentElement] = contentInfo;
@@ -1345,17 +1346,7 @@ XULSortServiceImpl::InsertContainerNode(nsIRDFCompositeDataSource *db, nsRDFSort
   if (sortInfo.db && sortInfo.naturalOrderSort) {
     // walk up the content model to find the REAL
     // parent container to determine if its a RDF_Seq
-    nsCOMPtr<nsIContent> parent = do_QueryInterface(container, &rv);
-    nsCOMPtr<nsIContent> aContent;
-
-    nsCOMPtr<nsIDocument> doc;
-    if (NS_SUCCEEDED(rv) && parent) {
-      doc = parent->GetDocument();
-      if (!doc)
-        parent = nsnull;
-    }
-
-    if (parent) {
+    if (container) {
       nsAutoString id;
 
       trueParent->GetAttr(kNameSpaceID_None, nsXULAtoms::ref, id);
@@ -1364,7 +1355,7 @@ XULSortServiceImpl::InsertContainerNode(nsIRDFCompositeDataSource *db, nsRDFSort
 
       if (!id.IsEmpty()) {
         nsCOMPtr<nsIRDFResource> containerRes;
-        rv = nsXULContentUtils::MakeElementResource(doc, id, getter_AddRefs(containerRes));
+        gRDFService->GetUnicodeResource(id, getter_AddRefs(containerRes));
         if (NS_SUCCEEDED(rv))
           rv = gRDFC->IsSeq(sortInfo.db, containerRes,  &isContainerRDFSeq);
       }