279155 - Random Cleanup in nsXULElement.cpp

Status: RESOLVED FIXED

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

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Not sure if i should file this against DOM or XUL now that XUL is more
integrated with the rest of our DOM.

In any case, there's a lot of really stupid code in nsXULElement.cpp. This is a
patch to clean up at least some of it. Mostly stuff like using atoms rather then
strings, nsIContent rather then nsIDOMNode, and use macros to reduce
source-duplication.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 171907 [details] [diff] [review]
Patch to fix

- In IsEventHandler(nsIAtom* aName) (in nsXULElement.cpp):

     const char* name;
     aName->GetUTF8String(&name);

     if (name[0] != 'o' || name[1] != 'n') {
	 return PR_FALSE;
     }
+    
+    return aName == nsLayoutAtoms::onclick	    ||
+	    aName == nsLayoutAtoms::ondblclick	    ||
+	    aName == nsLayoutAtoms::onmousedown     ||
...

It'd be cool if we could combine this list of atom compares with the almost
identical list of atom compares in nsGenericHTMLElement.cpp.

sr=jst whether you do that or not.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I think i'd prefer to do that separatly. There's more code that can be shared
between nsGenericHTMLElement and nsXULElement, so if we figure out a good way to
do that lets do that then.

Comment 4

[Oliver Klee]

This comment still has the old variable name in it:

   /*
    * If aChild doesn't have children it can't be our ancestor
    */
-  if (aChild->GetChildCount() == 0) {
+  if (aPossibleAncestor->GetChildCount() == 0) {

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in and fixed the comment. Thanks for review!

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

...and marking fixed

Comment 7

[Scott MacGregor]

This bug introduced a smoketest blocker. Mail compose now crashes when you
attempt to auto complete an address. I filed a blocker bug: Bug #279788. Please
take a look at this as soon as possible. Thanks.