875328 - class Attr has a stranded "AppendChildTo" override (which no longer overrides)

Status: RESOLVED FIXED

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

Description

[Daniel Holbert [:dholbert]]

The class "Attr" has an attempted override for AppendChildTo() under "// nsINode interface":
> 67 virtual nsresult AppendChildTo(nsIContent* aKid, bool aNotify);
http://mxr.mozilla.org/mozilla-central/source/content/base/src/Attr.h?mark=67-67#59

HOWEVER this method doesn't actually override anything (and hence probably never gets invoked), since AppendChildTo is **not declared as virtual** in nsINode:
> 574   nsresult AppendChildTo(nsIContent* aKid, bool aNotify)
> 575   {
> 576     return InsertChildAt(aKid, GetChildCount(), aNotify);
> 577   }
http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsINode.h#574

This changed in bug 346744, which de-virtualized this method, but left the "override" on Attr around. (though at that point, the Attr class was named nsDOMAttribute).

This actually doesn't affect behavior, though, because Attr returns NS_ERROR_NOT_IMPLEMENTED both from its fake-AppendChildTo-override (which is never invoked) and also from its InsertChildAt override (which *is* invoked, per the quoted chunk of nsINode above.)

So we should be able to just drop Attr::AppendChildTo and call it a day.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Daniel Holbert [:dholbert]]

For the record: I discovered this bug this from skimming over :Six's script-generated patch on bug 856822, which surprisingly (and correctly) left this method untouched, when annotating actually-overriding methods in class Attr {} as MOZ_OVERRIDE.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 753292 [details] [diff] [review]
fix

r=me

Comment 4

[Daniel Holbert [:dholbert]]

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f17ab67ef7f0

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f17ab67ef7f0