deCOMify nsIStyledContent::GetID (make it return we a weak pointer to the ID

atom).  Bug 244249, patch by jpl24 <jlurz24@gmail.com>, r=sicking, sr=bzbarsky

content/base/public/nsIStyleRuleProcessor.h

@@ -83,7 +83,7 @@ struct RuleProcessorData {
   nsIContent*       mScopedRoot;    // Root of scoped stylesheet (set and unset by the supplier of the scoped stylesheet
   
   nsIAtom*          mContentTag;    // if content, then content->GetTag()
-  nsIAtom*          mContentID;     // if styled content, then styledcontent->GetID()
+  nsIAtom*          mContentID;     // if styled content, then weak reference to styledcontent->GetID()
   nsIStyledContent* mStyledContent; // if content, content->QI(nsIStyledContent)
   PRPackedBool      mIsHTMLContent; // if content, then does QI on HTMLContent, true or false
   PRPackedBool      mIsHTMLLink;    // if content, calls nsStyleUtil::IsHTMLLink

content/base/public/nsIStyledContent.h

@@ -60,7 +60,7 @@ public:
   // corresponds to the attribute nsHTMLAtoms::id and that the Class
   // corresponds to the attribute nsHTMLAtoms::kClass.  If this becomes
   // incorrect, then new methods need to be added here.
-  NS_IMETHOD GetID(nsIAtom** aResult) const = 0;
+  virtual nsIAtom* GetID() const = 0;
   virtual const nsAttrValue* GetClasses() const = 0;
   NS_IMETHOD_(PRBool) HasClass(nsIAtom* aClass, PRBool aCaseSensitive) const = 0;
 

content/base/src/nsGenericElement.cpp

@@ -2114,12 +2114,32 @@ nsGenericElement::MaybeTriggerAutoLink(nsIDocShell *aShell)
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsGenericElement::GetID(nsIAtom** aResult) const
+nsIAtom*
+nsGenericElement::GetID() const
 {
-  *aResult = nsnull;
+  nsIAtom* IDName = GetIDAttributeName();
+  if (IDName) {
+    const nsAttrValue* attrVal = mAttrsAndChildren.GetAttr(IDName);
+    if (attrVal){
+      if (attrVal->Type() == nsAttrValue::eAtom) {
+        return attrVal->GetAtomValue();
+      }
+      if(attrVal->IsEmptyString()){
+        return nsnull;
+      }
+      // Check if the ID has been stored as a string.
+      // This would occur if the ID attribute name changed after 
+      // the ID was parsed. 
+      if (attrVal->Type() == nsAttrValue::eString) {
+        nsAutoString idVal(attrVal->GetStringValue());
 
-  return NS_OK;
+        // Create an atom from the value and set it into the attribute list. 
+        NS_CONST_CAST(nsAttrValue*, attrVal)->ParseAtom(idVal);
+        return attrVal->GetAtomValue();
+      }
+    }
+  }
+  return nsnull;
 }
 
 const nsAttrValue*
@@ -3370,8 +3390,18 @@ nsGenericElement::SetAttr(PRInt32 aNamespaceID, nsIAtom* aName,
 
   nsresult rv;
   if (aNamespaceID == kNameSpaceID_None) {
-    rv = mAttrsAndChildren.SetAttr(aName, aValue);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (aName == GetIDAttributeName() && !aValue.IsEmpty()) {
+      // Store id as atom. id="" means that the element has no id, not that it has
+      // an emptystring as the id.
+      nsAttrValue attrValue;
+      attrValue.ParseAtom(aValue);
+      rv = mAttrsAndChildren.SetAndTakeAttr(aName, attrValue);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    else {
+      rv = mAttrsAndChildren.SetAttr(aName, aValue);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
   else {
     nsCOMPtr<nsINodeInfo> ni;
@@ -3564,7 +3594,7 @@ nsGenericElement::List(FILE* out, PRInt32 aIndent) const
 {
   NS_PRECONDITION(IsInDoc(), "bad content");
 
-  PRInt32 index;
+  PRUint32 index;
   for (index = aIndent; --index >= 0; ) fputs("  ", out);
 
   nsAutoString buf;
@@ -3593,7 +3623,7 @@ nsGenericElement::List(FILE* out, PRInt32 aIndent) const
   fprintf(out, " refcount=%d<", mRefCnt.get());
 
   fputs("\n", out);
-  PRInt32 kids = GetChildCount();
+  PRUint32 kids = GetChildCount();
 
   for (index = 0; index < kids; index++) {
     nsIContent *kid = GetChildAt(index);

content/base/src/nsGenericElement.h

@@ -429,7 +429,7 @@ public:
 #endif
 
   // nsIStyledContent interface methods
-  NS_IMETHOD GetID(nsIAtom** aResult) const;
+  virtual nsIAtom* GetID() const;
   virtual const nsAttrValue* GetClasses() const;
   NS_IMETHOD_(PRBool) HasClass(nsIAtom* aClass, PRBool aCaseSensitive) const;
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker);

content/events/src/nsXMLEventsManager.cpp

@@ -233,12 +233,8 @@ nsXMLEventsListener::HandleEvent(nsIDOMEvent* aEvent)
     nsCOMPtr<nsIDOMEventTarget> target;
     aEvent->GetTarget(getter_AddRefs(target));
     nsCOMPtr<nsIStyledContent> targetEl(do_QueryInterface(target));
-    if (targetEl) {
-      nsCOMPtr<nsIAtom> id;
-      targetEl->GetID(getter_AddRefs(id));
-      if (id == mTarget) 
+    if (targetEl && targetEl->GetID() == mTarget) 
         targetMatched = PR_TRUE;
-    }
   }
   if (!targetMatched)
     return NS_OK;

content/html/content/src/nsGenericHTMLElement.cpp

@@ -1882,24 +1882,6 @@ nsGenericHTMLElement::GetHTMLAttribute(nsIAtom* aAttribute,
   return NS_CONTENT_ATTR_HAS_VALUE;
 }
 
-nsresult
-nsGenericHTMLElement::GetID(nsIAtom** aResult) const
-{
-  *aResult = nsnull;
-
-  const nsAttrValue* attrVal = mAttrsAndChildren.GetAttr(nsHTMLAtoms::id);
-  NS_ASSERTION(!attrVal ||
-               attrVal->Type() == nsAttrValue::eAtom ||
-               (attrVal->Type() == nsAttrValue::eString &&
-                attrVal->GetStringValue().IsEmpty()),
-               "unexpected attribute type");
-  if (attrVal && attrVal->Type() == nsAttrValue::eAtom) {
-    NS_ADDREF(*aResult = attrVal->GetAtomValue());
-  }
-
-  return NS_OK;
-}
-
 const nsAttrValue*
 nsGenericHTMLElement::GetClasses() const
 {

content/html/content/src/nsGenericHTMLElement.h

@@ -211,7 +211,6 @@ public:
 
   // Implementation for nsIHTMLContent
   NS_IMETHOD GetHTMLAttribute(nsIAtom* aAttribute, nsHTMLValue& aValue) const;
-  NS_IMETHOD GetID(nsIAtom** aResult) const;
   virtual const nsAttrValue* GetClasses() const;
   virtual nsIAtom *GetIDAttributeName() const;
   virtual nsIAtom *GetClassAttributeName() const;

content/html/style/src/nsCSSStyleSheet.cpp

@@ -2859,7 +2859,7 @@ RuleProcessorData::RuleProcessorData(nsPresContext* aPresContext,
     // get the styledcontent interface and the ID
     if (NS_SUCCEEDED(aContent->QueryInterface(NS_GET_IID(nsIStyledContent), (void**)&mStyledContent))) {
       NS_ASSERTION(mStyledContent, "Succeeded but returned null");
-      mStyledContent->GetID(&mContentID);
+      mContentID = mStyledContent->GetID();
     }
 
     // see if there are attributes for the content
@@ -2910,7 +2910,6 @@ RuleProcessorData::~RuleProcessorData()
   if (mParentData)
     mParentData->Destroy(mPresContext);
 
-  NS_IF_RELEASE(mContentID);
   NS_IF_RELEASE(mStyledContent);
 
   delete mLanguage;

content/svg/content/src/nsSVGElement.cpp

@@ -202,8 +202,11 @@ nsSVGElement::SetAttr(PRInt32 aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix,
   else if (aName == nsSVGAtoms::style && aNamespaceID == kNameSpaceID_None) {
     nsGenericHTMLElement::ParseStyleAttribute(this, PR_TRUE, aValue, attrValue);
   }
+  // We don't have an nsISVGValue attribute.
+  else if (aName == nsSVGAtoms::id && aNamespaceID == kNameSpaceID_None){
+    attrValue.ParseAtom(aValue);
+  } 
   else {
-    // We don't have an nsISVGValue attribute.
     attrValue.SetTo(aValue);
   }
 
@@ -278,21 +281,6 @@ nsSVGElement::IsContentOfType(PRUint32 aFlags) const
 //----------------------------------------------------------------------
 // nsIStyledContent methods
 
-NS_IMETHODIMP
-nsSVGElement::GetID(nsIAtom** aId)const
-{
-  nsAutoString value;
-  
-  nsresult rv = NS_CONST_CAST(nsSVGElement*,this)->
-                    GetAttribute(NS_LITERAL_STRING("id"), value);
-  if (NS_SUCCEEDED(rv))
-    *aId = NS_NewAtom(value);
-  else
-    *aId = nsnull;
-  
-  return rv;
-}
-
 NS_IMETHODIMP
 nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {

content/svg/content/src/nsSVGElement.h

@@ -85,9 +85,6 @@ public:
   
   virtual nsresult SetBindingParent(nsIContent* aParent);
   virtual PRBool IsContentOfType(PRUint32 aFlags) const;
-
-  // nsIStyledContent
-  NS_IMETHOD GetID(nsIAtom** aResult) const;
   
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker);
   NS_IMETHOD SetInlineStyleRule(nsICSSStyleRule* aStyleRule, PRBool aNotify);

content/xml/content/src/nsXMLElement.cpp

@@ -437,22 +437,3 @@ nsXMLElement::IsFocusable(PRInt32 *aTabIndex)
 
 NS_IMPL_DOM_CLONENODE(nsXMLElement)
 
-// nsIStyledContent implementation
-
-NS_IMETHODIMP
-nsXMLElement::GetID(nsIAtom** aResult) const
-{
-  nsIAtom* atom = GetIDAttributeName();
-
-  *aResult = nsnull;
-  nsresult rv = NS_OK;
-  if (atom) {
-    nsAutoString value;
-    rv = nsGenericElement::GetAttr(kNameSpaceID_None, atom, value);
-    if (NS_SUCCEEDED(rv)) {
-      *aResult = NS_NewAtom(value);
-    }
-  }
-
-  return rv;
-}

content/xml/content/src/nsXMLElement.h

@@ -69,9 +69,6 @@ public:
   // nsIXMLContent
   NS_IMETHOD MaybeTriggerAutoLink(nsIDocShell *aShell);
 
-  // nsIStyledContent
-  NS_IMETHOD GetID(nsIAtom** aResult) const;
-
   // nsIContent
   nsresult SetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
                    const nsAString& aValue, PRBool aNotify)

content/xml/document/src/nsXMLContentSink.cpp

@@ -1039,6 +1039,17 @@ nsXMLContentSink::HandleStartElement(const PRUnichar *aName,
   }
   content->SetDocument(mDocument, PR_FALSE, PR_TRUE);
 
+  // Set the ID attribute atom on the node info object for this node
+  // This must occur before the attributes are added so the name
+  // of the id attribute is known.
+  if (aIndex != -1 && NS_SUCCEEDED(result)) {
+    nsCOMPtr<nsIAtom> IDAttr = do_GetAtom(aAtts[aIndex]);
+
+    if (IDAttr) {
+      nodeInfo->SetIDAttributeAtom(IDAttr);
+    }
+  }
+  
   // Set the attributes on the new content element
   result = AddAttributes(aAtts, content);
 
@@ -1054,15 +1065,6 @@ nsXMLContentSink::HandleStartElement(const PRUnichar *aName,
     PushContent(content);
   }
 
-  // Set the ID attribute atom on the node info object for this node
-  if (aIndex != -1 && NS_SUCCEEDED(result)) {
-    nsCOMPtr<nsIAtom> IDAttr = do_GetAtom(aAtts[aIndex]);
-
-    if (IDAttr) {
-      nodeInfo->SetIDAttributeAtom(IDAttr);
-    }
-  }
-
 #ifdef MOZ_XTF
   if (nameSpaceID > kNameSpaceID_LastBuiltin)
     content->BeginAddingChildren();

content/xml/document/src/nsXMLDocument.cpp

@@ -735,9 +735,8 @@ MatchElementId(nsIContent *aContent, const nsACString& aUTF8Id, const nsAString&
     nsCOMPtr<nsIXMLContent> xmlContent = do_QueryInterface(aContent);    
 
     if (xmlContent) {
-      nsCOMPtr<nsIAtom> value;
-      if (NS_SUCCEEDED(xmlContent->GetID(getter_AddRefs(value))) &&
-          value && value->EqualsUTF8(aUTF8Id)) {
+      nsIAtom* value = xmlContent->GetID();
+      if (value && value->EqualsUTF8(aUTF8Id)) {
         return aContent;
       }
     }

content/xul/content/src/nsXULElement.cpp

@@ -3083,10 +3083,10 @@ nsXULElement::EnsureContentsGenerated(void) const
 }
 
 // nsIStyledContent Implementation
-NS_IMETHODIMP
-nsXULElement::GetID(nsIAtom** aResult) const
+/// XXX GetID must be defined here because nsXUL element does not inherit from nsGenericElement.
+nsIAtom*
+nsXULElement::GetID() const
 {
-    *aResult = nsnull;
     const nsAttrValue* attrVal = FindLocalOrProtoAttr(kNameSpaceID_None, nsXULAtoms::id);
 
     NS_ASSERTION(!attrVal ||
@@ -3096,10 +3096,9 @@ nsXULElement::GetID(nsIAtom** aResult) const
                  "unexpected attribute type");
 
     if (attrVal && attrVal->Type() == nsAttrValue::eAtom) {
-        NS_ADDREF(*aResult = attrVal->GetAtomValue());
+        return attrVal->GetAtomValue();
     }
-
-    return NS_OK;
+    return nsnull;
 }
 
 const nsAttrValue*

content/xul/content/src/nsXULElement.h

@@ -519,7 +519,7 @@ public:
     NS_IMETHOD MaybeTriggerAutoLink(nsIDocShell *aShell);
 
     // nsIStyledContent
-    NS_IMETHOD GetID(nsIAtom** aResult) const;
+    virtual nsIAtom* GetID() const;
     virtual const nsAttrValue* GetClasses() const;
     NS_IMETHOD_(PRBool) HasClass(nsIAtom* aClass, PRBool aCaseSensitive) const;
 

layout/style/nsCSSStyleSheet.cpp

@@ -2859,7 +2859,7 @@ RuleProcessorData::RuleProcessorData(nsPresContext* aPresContext,
     // get the styledcontent interface and the ID
     if (NS_SUCCEEDED(aContent->QueryInterface(NS_GET_IID(nsIStyledContent), (void**)&mStyledContent))) {
       NS_ASSERTION(mStyledContent, "Succeeded but returned null");
-      mStyledContent->GetID(&mContentID);
+      mContentID = mStyledContent->GetID();
     }
 
     // see if there are attributes for the content
@@ -2910,7 +2910,6 @@ RuleProcessorData::~RuleProcessorData()
   if (mParentData)
     mParentData->Destroy(mPresContext);
 
-  NS_IF_RELEASE(mContentID);
   NS_IF_RELEASE(mStyledContent);
 
   delete mLanguage;

layout/style/nsIStyleRuleProcessor.h

@@ -83,7 +83,7 @@ struct RuleProcessorData {
   nsIContent*       mScopedRoot;    // Root of scoped stylesheet (set and unset by the supplier of the scoped stylesheet
   
   nsIAtom*          mContentTag;    // if content, then content->GetTag()
-  nsIAtom*          mContentID;     // if styled content, then styledcontent->GetID()
+  nsIAtom*          mContentID;     // if styled content, then weak reference to styledcontent->GetID()
   nsIStyledContent* mStyledContent; // if content, content->QI(nsIStyledContent)
   PRPackedBool      mIsHTMLContent; // if content, then does QI on HTMLContent, true or false
   PRPackedBool      mIsHTMLLink;    // if content, calls nsStyleUtil::IsHTMLLink