Bug 11053 – XMLHttpRequest should be an EventTarget

• Component: XML
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

Objects implementing the XMLHttpRequest interface MUST also implement the EventTarget interface: <http://www.w3.org/TR/DOM-Level-3-Events/events.html#Events-EventTarget>.

[Alexey Proskuryakov]

Created attachment 11633 [details]
proposed patch

[Geoffrey Garen]

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?

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 11775 [details]
proposed patch

Used a typedef for the monstrosity.

[Darin Adler]

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

[Darin Adler]

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.

[Alexey Proskuryakov]

Created attachment 11815 [details]
proposed patch

More typedef usage.

[Darin Adler]

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) {

[Alexey Proskuryakov]

Committed revision 18172.