Bug 25345 – Make EventListener::virtualisAttribute pure virtual

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Misc.

[Dimitri Glazkov (Google)]

This has bitten us several times during the merge now, where renaming this method did not trigger any compile failures -- but had pretty severe consequences.

[Dimitri Glazkov (Google)]

Created attachment 29712 [details]
Change EventListener::virtualisAttribute to be pure virtual, v1.

 WebCore/ChangeLog                         |   17 +++++++++++++++++
 WebCore/bindings/objc/ObjCEventListener.h |    1 +
 WebCore/dom/EventListener.h               |    2 +-
 WebCore/loader/ImageDocument.cpp          |    1 +
 WebCore/svg/animation/SVGSMILElement.cpp  |    2 ++
 WebCore/xml/XPathResult.cpp               |    1 +
 6 files changed, 23 insertions(+), 1 deletions(-)

[Darin Adler]

Comment on attachment 29712 [details]
Change EventListener::virtualisAttribute to be pure virtual, v1.

The overrides should be private, like the function in the base class.

Also, it's a programming mistake to call isAttribute on a subclass object, so isAttribute should either be overridden to return false in those subclasses, or be made private by putting "using EventListener::isAttribute" in the private section of those classes.

Since this patch doesn't fix a bug, I'm going to say review- until you make the overrides private.

[Sam Weinig]

I don't see the point of this at all.  This adds complexity while not fixing any issue.

[Dimitri Glazkov (Google)]

Dave Levin suggested a simpler approach using COMPILE_ASSERT, which sounds good, too. I think I'll close this and go with that, instead.