Bug 25345 - Make EventListener::virtualisAttribute pure virtual
Summary: Make EventListener::virtualisAttribute pure virtual
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-23 09:53 PDT by Dimitri Glazkov (Google)
Modified: 2009-04-23 15:40 PDT (History)
4 users (show)

See Also:


Attachments
Change EventListener::virtualisAttribute to be pure virtual, v1. (3.63 KB, patch)
2009-04-23 09:54 PDT, Dimitri Glazkov (Google)
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2009-04-23 09:53:00 PDT
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.
Comment 1 Dimitri Glazkov (Google) 2009-04-23 09:54:40 PDT
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(-)
Comment 2 Darin Adler 2009-04-23 10:03:38 PDT
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.
Comment 3 Sam Weinig 2009-04-23 10:53:22 PDT
I don't see the point of this at all.  This adds complexity while not fixing any issue.
Comment 4 Dimitri Glazkov (Google) 2009-04-23 15:40:25 PDT
Dave Levin suggested a simpler approach using COMPILE_ASSERT, which sounds good, too. I think I'll close this and go with that, instead.