[webkit-reviews] review denied: [Bug 26141] Implement onformchange and onforminput event handlers : [Attachment 78643] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Dai Mikurube
<dmikurube at google.com>'s request for review:
Bug 26141: Implement onformchange and onforminput event handlers
https://bugs.webkit.org/show_bug.cgi?id=26141

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78643&action=review

Please see HTMLFormElement::collectUnhandledInvalidControls() for how to fix
the following dispatchEvent() issues.

>> LayoutTests/fast/forms/script-tests/formchange-event.js:3
>> +function sendKey(element, keyName) {
> 
> You should probably use eventSender here instead.

I like avoiding eventSender in order to make the test workable on browsers.

>> Source/WebCore/html/HTMLElement.cpp:828
>> +	dispatchEvent(Event::create(eventNames().inputEvent, true, false));
> 
> Node::dispatchInputEvents();

Because dispatchEvent() may call JavaScript event handlers, "this" object can
be deleted in the event handlers.

> Source/WebCore/html/HTMLFormControlElement.cpp:183
> +    dispatchEvent(Event::create(eventNames().inputEvent, true, false));

ditto.
"this" might be deleted in dispatchEvent().  We can't call any methods of this.


> Source/WebCore/html/HTMLFormElement.cpp:600
> +	   if
(!formElement->dispatchEvent(Event::create(eventNames().forminputEvent, false,
false)))

ditto.
dispatchEvent() calls JavaScript code.	"this" can be deleted and
m_associatedElements can be updated.

> Source/WebCore/html/HTMLFormElement.cpp:612
> +	   if
(!formElement->dispatchEvent(Event::create(eventNames().formchangeEvent, false,
false)))

ditto.