[webkit-reviews] review denied: [Bug 18510] Text controls only emit the start editing signal after the first keypress : [Attachment 20549] Patch to call textFieldDidStartEditing whenever a text control receives focus

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 18510: Text controls only emit the start editing signal after the first
keypress
http://bugs.webkit.org/show_bug.cgi?id=18510

Attachment 20549: Patch to call textFieldDidStartEditing whenever a text
control receives focus
http://bugs.webkit.org/attachment.cgi?id=20549&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Thanks for tackling this!

The indentation of the code looks wrong, and part of the patch is only this
indentation change on some existing code.

+	    HTMLInputElement* input =
static_cast<HTMLInputElement*>(shadowAncestor);

This cast is incorrect. As you can see above, the shadowAncestor might be a
HTMLTextAreaElement.

+	   
static_cast<RenderTextControl*>(shadowAncestor->renderer())->document()->frame(
)->textFieldDidBeginEditing(input);

Doing this without any null checks is almost certainly incorrect. It may be
possible for events to be dispatched in a document that no longer is in a
frame, for example. And the old code that's being replaced was doing the null
checks.

As with any WebKit bug fix, the patch needs to include a regression test that
demonstrates the problem. And a ChangeLog entry.