66835 - fix warnings from jst's nsGenericElement checkin

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm working on a patch to fix the warnings from jst's checkin making content
objects inherit from nsGenericElement, using |using|.  It seems to work
for me (RedHat gcc-2.96-70), and I'm using the
HAVE_CPP_AMBIGUITY_RESOLVING_USING macro set by autoconf and also set for
Mac.  I'm not sure whether the autoconf test is exactly what we want, though.
(It's not used anywhere else.)

Anyway, I'll attach a patch that shows the general approach.  I've fixed
many of the warnings that can be fixed by changing nsGenericElement
or nsGenericHTMLElement, but some can be fixed only on the different
content objects.  I've only done nsHTMLAnchorElement as a sample.

Any thoughts on whether this is a good idea?  It would sure be nice not to
have to look at these warnings...

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch so far...

Comment 2

[Prashant Desale]

QA contact Update

Comment 3

[Simon Fraser [no longer active]]

Will this fix these warnings? --

Warning : 'nsGenericElement::GetAttribute(const nsAString &, nsAString &)' hides 
inherited virtual function 'nsIContent::GetAttribute(int, nsIAtom *, nsAString &
)const '
nsGenericElement.h line 361   };

Warning : 'nsGenericElement::SetAttribute(const nsAString &, const nsAString &)' 
hides inherited virtual function 'nsIContent::SetAttribute(nsINodeInfo *, const 
nsAString &, int)'
nsGenericElement.h line 361   };

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes, it should.

I think jst said he has a different fix for this, and he's not sure which to use.
(I think his fix is renaming one of the two |SetAttribute|/|GetAttribute| pairs
of methods, but I'm not sure.)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Reality check.  Moving out to 0.9.1.

Comment 6

[Prashant Desale]

Updating QA contact to Shivakiran Tummala.

Comment 7

[Brian Ryner (not reading)]

Attachment: patch

Comment 8

[Brian Ryner (not reading)]

Ok, this patch fixes the method hiding warnings by changing Attribute to Attr in
nsIContent method names.

Comment 9

[jag (Peter Annema)]

r=jag

Comment 10

[Johnny Stenback (:jst)]

Bryner, you're my hero for fixing all these warnings I introduced way back when,
sr=jst for the changes, all I found when reviewing was a few indendtation nits:

- nsDOMAttributeMap.cpp:

@@ -291,7 +291,7 @@
     nsresult attrResult;
     nsAutoString value;
 
-    attrResult = mContent->GetAttribute(nameSpaceID, nameAtom,
+    attrResult = mContent->GetAttr(nameSpaceID, nameAtom,
                                         *getter_AddRefs(prefix), value);

nsGenericHTMLElement.cpp:

@@ -1583,7 +1583,7 @@
 
   // We still rely on the old way of setting the attribute.
 
-  return NS_STATIC_CAST(nsIContent *, this)->SetAttribute(nsid, atom, aValue,
+  return NS_STATIC_CAST(nsIContent *, this)->SetAttr(nsid, atom, aValue,
                                                           aNotify);
 
- nsXULContentBuilder.cpp:

 else {
-                aRealElement->UnsetAttribute(attribNameSpaceID,
+                aRealElement->UnsetAttr(attribNameSpaceID,
                                              attribName,
                                              PR_TRUE);
             }
@@ -1398,7 +1398,7 @@

- nsXULTemplateBuilder.cpp:

-        rv = aRuleElement->GetAttributeNameAt(i, attrNameSpaceID,
+        rv = aRuleElement->GetAttrNameAt(i, attrNameSpaceID,
                                               *getter_AddRefs(attr),
                                               *getter_AddRefs(prefix));
         if (NS_FAILED(rv)) return rv;
@@ -2152,7 +2152,7 @@

and also around line 2310:

-            element->GetAttributeNameAt(i, nameSpaceID, *getter_AddRefs(attr),
+            element->GetAttrNameAt(i, nameSpaceID, *getter_AddRefs(attr),
                                         *getter_AddRefs(prefix));

Comment 11

[Brian Ryner (not reading)]

I'll take this since I have the patch.

Comment 12

[Brian Ryner (not reading)]

Checked in.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Fixing database corruption caused by bug 95743.  Pay no attention to the man
behind the curtain.

Comment 14

[Sivakiran Tummala]

This is strictly a development issue, warning messages in the code are used for
debugging and does not have any greater impact. Marking verified as per
developer comments above.