Bug 29701 - Bring a little sanity to this crazy EventTarget world of ours
Summary: Bring a little sanity to this crazy EventTarget world of ours
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks: 29713
  Show dependency tree / graph
 
Reported: 2009-09-23 20:16 PDT by Geoffrey Garen
Modified: 2009-09-24 10:41 PDT (History)
0 users

See Also:

Attachments
swap vectors with inline capacities (2.54 KB, patch)
2009-09-23 20:17 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
layout test changes (14.36 KB, patch)
2009-09-23 20:18 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
webkit support (1.15 KB, patch)
2009-09-23 20:18 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
webcore patch - changelog in progress (325.08 KB, patch)
2009-09-23 20:57 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2009-09-23 20:16:58 PDT 
So crazy!
Comment 1 Geoffrey Garen 2009-09-23 20:17:38 PDT 
Created attachment 40037 [details]
swap vectors with inline capacities
Comment 2 Geoffrey Garen 2009-09-23 20:18:03 PDT 
Created attachment 40038 [details]
layout test changes
Comment 3 Geoffrey Garen 2009-09-23 20:18:43 PDT 
Created attachment 40039 [details]
webkit support
Comment 4 Geoffrey Garen 2009-09-23 20:57:18 PDT 
Created attachment 40042 [details]
webcore patch - changelog in progress
Comment 5 Sam Weinig 2009-09-23 21:39:38 PDT 
Comment on attachment 40042 [details]
webcore patch - changelog in progress

> +    if (!result.second) // pre-existing entry
> +        if (entry.find(registeredListener) != notFound) // duplicate listener
> +            return false;

The outer if statement needs braces.

> +
> +    // Notify firing events planning to invoke the listener at 'index' that
> +    // they have one less listener to invoke.
> +    for (size_t i = 0; i < d->firingEventEndIterators.size(); ++i)
> +        if (eventType == *d->firingEventEndIterators[i].eventType && index < *d->firingEventEndIterators[i].value)
> +            --*d->firingEventEndIterators[i].value;

The for-loop needs braces.

> +EventListener* EventTarget::getAttributeEventListener(const AtomicString& eventType)
> +{
> +    const EventListenerVector& entry = getEventListeners(eventType);
> +    for (size_t i = 0; i < entry.size(); ++i)
> +        if (entry[i].listener->isAttribute())
> +            return entry[i].listener.get();

For-loop needs braces.


> +
> +        bool fireEventListeners(Event*);
> +        bool isFiringEventListeners();
> +        bool dispatchEvent(PassRefPtr<Event>, ExceptionCode&); // DOM API
I think it makes sense to put this method up with addEventListener and removeEventListener.


> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(abort);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(change);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(click);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(contextmenu);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dblclick);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dragenter);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dragover);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dragleave);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(drop);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dragstart);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(drag);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(dragend);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(input);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(invalid);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(keydown);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(keypress);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(keyup);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mousedown);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mousemove);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mouseout);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mouseover);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mouseup);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(mousewheel);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(scroll);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(select);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(submit);
...
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(beforecut);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(cut);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(beforecopy);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(copy);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(beforepaste);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(paste);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(reset);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(search);
> +    DEFINE_ATTRIBUTE_EVENT_LISTENER(selectstart);

These should all be treated the same way as the 4 virtual ones and only on Element and Document.


> -    void setOnbeforeunload(PassRefPtr<EventListener>);
> -    EventListener* onmessage() const;
> -    void setOnmessage(PassRefPtr<EventListener>);
> -    EventListener* onhashchange() const;
> -    void setOnhashchange(PassRefPtr<EventListener>);
> -    EventListener* onoffline() const;
> -    void setOnoffline(PassRefPtr<EventListener>);
> -    EventListener* ononline() const;
> -    void setOnonline(PassRefPtr<EventListener>);
> +    // Declared virtual in Node

Should be // Declared virtual in Element

> -
> -    EventListener* onbeforeunload() const;
> -    void setOnbeforeunload(PassRefPtr<EventListener>);
> -    EventListener* onhashchange() const;
> -    void setOnhashchange(PassRefPtr<EventListener>);
> -    EventListener* onmessage() const;
> -    void setOnmessage(PassRefPtr<EventListener>);
> -    EventListener* onoffline() const;
> -    void setOnoffline(PassRefPtr<EventListener>);
> -    EventListener* ononline() const;
> -    void setOnonline(PassRefPtr<EventListener>);
> +    // Declared virtual in Node

Should be // Declared virtual in Element


Since these issues are mainly superficial, r=me.  But please make them :).