Closed Bug 811449 Opened 7 years ago Closed 7 years ago

Merge nsGenericElement and Element

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(8 files, 1 obsolete file)

117.62 KB, patch
peterv
: review+
Details | Diff | Splinter Review
6.10 KB, patch
peterv
: review+
Details | Diff | Splinter Review
21.91 KB, patch
peterv
: review+
Details | Diff | Splinter Review
1.58 KB, patch
peterv
: review+
Details | Diff | Splinter Review
8.15 KB, patch
peterv
: review+
Details | Diff | Splinter Review
1.12 KB, patch
peterv
: review+
Details | Diff | Splinter Review
78.99 KB, patch
peterv
: review+
Details | Diff | Splinter Review
191.76 KB, patch
peterv
: review+
Details | Diff | Splinter Review
Reducing the ugly.
It probably doesn't need review yet ;)
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [need review]
Version: unspecified → Trunk
outside of the Link bits that are needed to make mock_Link compile.
Attachment #681386 - Flags: review?(peterv)
Attachment #681394 - Flags: review?(peterv)
Attachment #681401 - Flags: review?(peterv)
Attachment #681394 - Attachment is obsolete: true
Attachment #681394 - Flags: review?(peterv)
Attachment #681401 - Attachment description: Part 7, but not crashing → Part 8, but not crashing
Whiteboard: [need review]
Comment on attachment 681386 [details] [diff] [review]
part 1.  Move everything from nsGenericElement.h to Element.h.   This is just moving code as-is, with absolutely no changes

Review of attachment 681386 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/public/Element.h
@@ +412,5 @@
> +class nsDOMTokenList;
> +struct nsRect;
> +
> +already_AddRefed<nsContentList>
> +NS_GetContentList(nsINode* aRootNode, 

Trailing whitespace.
Attachment #681386 - Flags: review?(peterv) → review+
Comment on attachment 681388 [details] [diff] [review]
part 2.  Combine the forward-declares and includes in Element.h.   is just moving includes and forward-declares and some inline

Review of attachment 681388 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/public/Element.h
@@ +43,5 @@
> +#include "nsIScrollableFrame.h"
> +#include "nsIDOMAttr.h"
> +#include "nsISMILAttr.h"
> +#include "nsClientRect.h"
> +#include "nsIDOMDOMTokenList.h"

Just sort those alphabetically while you're taking blame :)

@@ +67,5 @@
>  class nsEventStateManager;
>  class nsFocusManager;
>  class nsGlobalWindow;
>  class nsICSSDeclaration;
>  class nsISMILAttr;

And hey, these too
Comment on attachment 681393 [details] [diff] [review]
part 7.  Get rid of nsGenericElement in Element.cpp.

Review of attachment 681393 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/Element.cpp
@@ +2513,1 @@
>                                       ErrorResult& aError)

Missed this

@@ +2542,1 @@
>                                   nsAttrValue& aResult)

And this
Attachment #681388 - Flags: review?(peterv) → review+
Attachment #681389 - Flags: review?(peterv) → review+
Attachment #681390 - Flags: review?(peterv) → review+
Attachment #681391 - Flags: review?(peterv) → review+
Attachment #681392 - Flags: review?(peterv) → review+
> Just sort those alphabetically while you're taking blame :)

I'd really rather not, actually.  It increases risk, unfortunately.

Fixed the two indentation bits.
Attachment #681393 - Flags: review?(peterv) → review+
Comment on attachment 681401 [details] [diff] [review]
Part 8, but not crashing

Review of attachment 681401 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsDOMTokenList.h
@@ +34,1 @@
>    typedef mozilla::dom::Element Element;

As discussed, let's make this protected.

::: content/base/src/nsStyledElement.cpp
@@ +129,1 @@
>                                          aNotify);

Rewrap.

::: content/html/content/src/nsHTMLFrameSetElement.cpp
@@ +21,5 @@
>  }
>  
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLFrameSetElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLFrameSetElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLHRElement.cpp
@@ +67,5 @@
>  }
>  
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLHRElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLHRElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLSharedObjectElement.cpp
@@ +210,5 @@
>    nsObjectLoadingContent::Traverse(tmp, cb);
>  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLSharedObjectElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLSharedObjectElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLStyleElement.cpp
@@ +118,5 @@
>    tmp->nsStyleLinkElement::Unlink();
>  NS_IMPL_CYCLE_COLLECTION_UNLINK_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLStyleElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLStyleElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTableCellElement.cpp
@@ +78,5 @@
>  }
>  
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTableCellElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTableCellElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTableColElement.cpp
@@ +71,5 @@
>  }
>  
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTableColElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTableColElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTableElement.cpp
@@ +357,5 @@
>    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mRows)
>  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTableElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTableElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTableRowElement.cpp
@@ +83,5 @@
>                                                         nsIDOMNodeList)
>  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTableRowElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTableRowElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTableSectionElement.cpp
@@ +79,5 @@
>                                                         nsIDOMNodeList)
>  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTableSectionElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTableSectionElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTextAreaElement.cpp
@@ +310,5 @@
>    tmp->mState.Traverse(cb);
>  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTextAreaElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTextAreaElement, Element) 

Trailing whitespace.

::: content/html/content/src/nsHTMLTitleElement.cpp
@@ +77,5 @@
>  }
>  
>  
> +NS_IMPL_ADDREF_INHERITED(nsHTMLTitleElement, Element) 
> +NS_IMPL_RELEASE_INHERITED(nsHTMLTitleElement, Element) 

Trailing whitespace.
Attachment #681401 - Flags: review?(peterv) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.