[Webkit-unassigned] [Bug 60269] Ability to add C++ event listeners to html dom elements and dom window

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60269





--- Comment #1 from Brent Fulgham <bfulgham at webkit.org>  2011-05-05 11:30:32 PST ---
(From update of attachment 92420)
View in context: https://bugs.webkit.org/attachment.cgi?id=92420&action=review

> Source/WebKit/win/DOMCoreClasses.cpp:409
> +    RefPtr<CPPEventListener> cpplistener = adoptRef(new CPPEventListener(listener));

I don't think it's necessary to prefix the classname with CPP.  Maybe call it something like "CustomEventListener" or "UserEventListener", or even "EventListenerAdapter"?

> Source/WebKit/win/DOMCoreClasses.cpp:423
> +    m_node->removeEventListener(type, (WebCore::EventListener*)cpplistener.get(), useCapture);

You should use a C++ cast here.

> Source/WebKit/win/DOMCoreClasses.cpp:894
> +    RefPtr<CPPEventListener> cppListener = adoptRef(new CPPEventListener(listener));

Ditto re: naming.

> Source/WebKit/win/DOMCoreClasses.cpp:907
> +    m_window->removeEventListener(type, (WebCore::EventListener*)cpplistener.get(), useCapture);

Ditto re: Casting

> Source/WebKit/win/DOMCoreClasses.cpp:916
> +        return E_FAIL;

Shouldn't you be checking result as well?  I'd check evt & result for null and return E_POINTER, and return E_FAIL if m_window is null.

> Source/WebKit/win/DOMCoreClasses.cpp:919
> +    if (![self _node]->isEventTargetNode())

Do we have a C++ analog for this call?  Which DOM object are we expecting to get here?

> Source/WebKit/win/DOMCoreClasses.cpp:931
> +    WebCore::raiseOnDOMError(ec);

Is this not implemented for the C++ backend? I'm not sure why it's commented out, since we should be getting an exception code in the previous statement.

> Source/WebKit/win/DOMCoreClasses.cpp:957
> +    hr = newWindow->QueryInterface(IID_IDOMWindow, (void**)&domWindow);

This should be a C++ cast, even though others in the file are still using C-style casts.

> Source/WebKit/win/DOMEventsClasses.cpp:74
> +    DOMUIEvent * domUIEvent = new DOMUIEvent(ePtr);

This would be better as a COMPtr (see #include <WebCore/COMPtr.h>).

> Source/WebKit/win/DOMEventsClasses.cpp:76
> +    IDOMEvent* iDOMUIEvent = 0;

Ditto regarding COMPtr.

> Source/WebKit/win/DOMEventsClasses.cpp:77
> +    domUIEvent->QueryInterface(IID_IDOMEvent, (void**)&iDOMUIEvent);

This should be a C++-style cast.

> Source/WebKit/win/Interfaces/IWebFrame.idl:104
> +

Note: You need to add new methods at the bottom of the IDL file to avoid breaking existing Windows clients.

> Source/WebKit/win/WebFrame.cpp:483
> +HRESULT STDMETHODCALLTYPE WebFrame::DOMWindow(

You don't need to declare the STDMETHODCALLTYPE here (it's in the header), even though other methods in this file use this redundant declaration.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.