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

RESOLVED FIXED in mozilla24

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 753292 [details] [diff] [review]
fix
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #753292 - Flags: review?(bzbarsky)
(Assignee)

Comment 2

5 years ago
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 on attachment 753292 [details] [diff] [review]
fix

r=me
Attachment #753292 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

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

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/f17ab67ef7f0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.