[webkit-reviews] review denied: [Bug 43778] Dropping should fire textInput event : [Attachment 64070] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 11 10:37:06 PDT 2010


Tony Chang <tony at chromium.org> has denied MORITA Hajime <morrita at google.com>'s
request for review:
Bug 43778: Dropping should fire textInput event
https://bugs.webkit.org/show_bug.cgi?id=43778

Attachment 64070: Patch
https://bugs.webkit.org/attachment.cgi?id=64070&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +	   This chagne:

Nit: typo "change"

> +PassRefPtr<TextEvent> TextEvent::createForDrop(PassRefPtr<AbstractView>
view, const String& data)
> +{
> +    return adoptRef(new TextEvent(view, data, 0, false, false, false,
true));
> +}

There are too many bools here.	Can you first write a patch to convert these to
enums to make it more readable?

> diff --git a/WebCore/page/DragController.cpp
b/WebCore/page/DragController.cpp
> @@ -376,6 +386,9 @@ bool DragController::concludeEditDrag(DragData* dragData)

>      Frame* innerFrame = element->ownerDocument()->frame();
>      ASSERT(innerFrame);
>  
> +    if (!dispatchTextInputEventFor(innerFrame, dragData))
> +	   return true;

I think it's possible for element to be null here (e.g., if the event handler
removed the node from the DOM).  Can you add a test case for this?  We should
do the same for the paste event.


More information about the webkit-reviews mailing list