Bug 29701 – Bring a little sanity to this crazy EventTarget world of ours

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Geoffrey Garen]

So crazy!

[Geoffrey Garen]

Created attachment 40037 [details]
swap vectors with inline capacities

[Geoffrey Garen]

Created attachment 40038 [details]
layout test changes

[Geoffrey Garen]

Created attachment 40039 [details]
webkit support

[Geoffrey Garen]

Created attachment 40042 [details]
webcore patch - changelog in progress

[Sam Weinig]

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 :).

[Geoffrey Garen]

http://trac.webkit.org/changeset/48701
http://trac.webkit.org/changeset/48703
http://trac.webkit.org/changeset/48704
http://trac.webkit.org/changeset/48705
http://trac.webkit.org/changeset/48706
http://trac.webkit.org/changeset/48707