[webkit-reviews] review requested: [Bug 14111] Autogenerate Event JS binding : [Attachment 15042] Update patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 14 23:38:31 PDT 2007


Sam Weinig <sam at webkit.org> has asked  for review:
Bug 14111: Autogenerate Event JS binding
http://bugs.webkit.org/show_bug.cgi?id=14111

Attachment 15042: Update patch
http://bugs.webkit.org/attachment.cgi?id=15042&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
>Is there no way to avoid special-casing clipboardData and dataTransfer? Can't
>we just move those to the appropriate event classes?

I have moved dataTransfer into MouseEvent (though perhaps it should be in a
DragEvent class that we don't have) but have to special-case clipboardData for
now as we have no JSClipboardEvent event class to push it into yet.  If you
think it is appropriate, I can create one and put the method there.

>+    // FIXME: Is this lock necessary?
>+    KJS::JSLock lock;
>
> Seems clear that it is.

Oops, didn't mean to leave that in,  Removed.

>+	      if (WebCore::JSUnprotectedEventListener* listener =
>static_cast<WebCore::JSUnprotectedEventListener*>(m_impl->onReadyStateChangeLi
stener()))
>Why is the WebCore prefix needed?

Fixed.

>-	  [OldStyleObjC] void initEvent(in DOMString eventTypeArg, 
>+	  [OldStyleObjC] void initEvent(in AtomicString eventTypeArg, 
>
>This seems wrong. The API should be DOMString even though the implementation
is
>AtomicString.

Fixed.



More information about the webkit-reviews mailing list