Bug 11053 - XMLHttpRequest should be an EventTarget
Summary: XMLHttpRequest should be an EventTarget
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL: http://dev.w3.org/cvsweb/~checkout~/2...
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-27 04:25 PDT by Alexey Proskuryakov
Modified: 2006-12-12 00:16 PST (History)
0 users

See Also:


Attachments
proposed patch (13.86 KB, patch)
2006-11-26 02:07 PST, Alexey Proskuryakov
ggaren: review-
Details | Formatted Diff | Diff
proposed patch (13.90 KB, patch)
2006-12-08 09:57 PST, Alexey Proskuryakov
darin: review-
Details | Formatted Diff | Diff
proposed patch (13.75 KB, patch)
2006-12-11 23:55 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2006-09-27 04:25:38 PDT
Objects implementing the XMLHttpRequest interface MUST also implement the EventTarget interface: <http://www.w3.org/TR/DOM-Level-3-Events/events.html#Events-EventTarget>.
Comment 1 Alexey Proskuryakov 2006-11-26 02:07:37 PST
Created attachment 11633 [details]
proposed patch
Comment 2 Geoffrey Garen 2006-12-07 15:45:10 PST
Comment on attachment 11633 [details]
proposed patch

+ HashMap<AtomicStringImpl*, Vector<RefPtr<EventListener> > >

Pleae use a typedef for this monstrosity.

There are subtle differences between the EventTarget implementation here and the one in EventTargetNode. It would be nice to have a common base class that both EventTargetNode and XMLHttpRequest could inherit from ("EventTarget?"). EventTargetNode would have to supplement its behavior -- to support listeners on disconnected nodes, for example -- but the base functionality of tracking event handlers would be identical. This would prevent bugs due to subtle differences in implementation from creeping in, and improve readability. What do you think?
Comment 3 Alexey Proskuryakov 2006-12-07 21:30:25 PST
I absolutely agree, and plan to do that when fixing bug 11610. I don't want to do it in this patch, because that would change existing behavior for onreadystatechange/onload listeners.
Comment 4 Alexey Proskuryakov 2006-12-08 09:57:07 PST
Created attachment 11775 [details]
proposed patch

Used a typedef for the monstrosity.
Comment 5 Darin Adler 2006-12-11 13:39:14 PST
Comment on attachment 11775 [details]
proposed patch

+    HashMap<AtomicStringImpl*, Vector<RefPtr<EventListener> > >& eventListeners = m_impl->eventListeners();

Should use the new typedef (and maybe make a local typedef to get it out of the class).
Comment 6 Darin Adler 2006-12-11 21:29:33 PST
Comment on attachment 11775 [details]
proposed patch

Lets get some more typedef action in JSXMLHttpRequest::mark and perhaps all the occurences of Vector<RefPtr<EventListener> > before we land this.
Comment 7 Alexey Proskuryakov 2006-12-11 23:55:27 PST
Created attachment 11815 [details]
proposed patch

More typedef usage.
Comment 8 Darin Adler 2006-12-11 23:59:03 PST
Comment on attachment 11815 [details]
proposed patch

r=me

+    typedef HashMap<AtomicStringImpl*,  ListenerVector> EventListenersMap;

Two spaces here.

For this code:

+    XMLHttpRequest::EventListenersMap& eventListeners = m_impl->eventListeners();
+    for (XMLHttpRequest::EventListenersMap::iterator mapIter = eventListeners.begin(); mapIter != eventListeners.end(); ++mapIter) {
+        for (XMLHttpRequest::ListenerVector::iterator vecIter = mapIter->second.begin(); vecIter != mapIter->second.end(); ++vecIter) {

I'd do it like this:

    typedef XMLHttpRequest::EventListenersMap ListenerMap;
    typedef XMLHttpRequest:: ListenerVector ListenerVector;
    ListenerMap& eventListeners = m_impl->eventListeners();
    for (ListenerMap::iterator mapIter = eventListeners.begin(); mapIter != eventListeners.end(); ++mapIter) {
        for (ListenerVector::iterator vecIter = mapIter->second.begin(); vecIter != mapIter->second.end(); ++vecIter) {
Comment 9 Alexey Proskuryakov 2006-12-12 00:16:56 PST
Committed revision 18172.