RESOLVED FIXED 29701
Bring a little sanity to this crazy EventTarget world of ours 
https://bugs.webkit.org/show_bug.cgi?id=29701
Summary Bring a little sanity to this crazy EventTarget world of ours
Geoffrey Garen
Reported 2009-09-23 20:16:58 PDT
So crazy!
Attachments
swap vectors with inline capacities (2.54 KB, patch)
2009-09-23 20:17 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
DetailsFormatted DiffDiff
layout test changes (14.36 KB, patch)
2009-09-23 20:18 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
DetailsFormatted DiffDiff
webkit support (1.15 KB, patch)
2009-09-23 20:18 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
DetailsFormatted DiffDiff
webcore patch - changelog in progress (325.08 KB, patch)
2009-09-23 20:57 PDT, Geoffrey Garen 		sam: review+
ggaren: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2009-09-23 20:17:38 PDT
Created attachment 40037 [details] swap vectors with inline capacities
Geoffrey Garen
Comment 2 2009-09-23 20:18:03 PDT
Created attachment 40038 [details] layout test changes
Geoffrey Garen
Comment 3 2009-09-23 20:18:43 PDT
Created attachment 40039 [details] webkit support
Geoffrey Garen
Comment 4 2009-09-23 20:57:18 PDT
Created attachment 40042 [details] webcore patch - changelog in progress
Sam Weinig
Comment 5 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 :).
Note You need to log in before you can comment on or make changes to this bug.