RESOLVED INVALID 18510
Text controls only emit the start editing signal after the first keypress
https://bugs.webkit.org/show_bug.cgi?id=18510
Summary Text controls only emit the start editing signal after the first keypress
iain
Reported 2008-04-15 04:22:44 PDT
I'm writing a backend for Clutter and I was hooking up an onscreen keyboard to WebKit so that it would area when the user selected a text control. I was using EditorClient::textFieldDidBeginEditing() to control when the keyboard appeared, but this method is only called once the user has entered something in the text area. There we get a chicken/egg problem, the keyboard can't appear until the user types something, but the user cant type something without the keyboard. Attached is a patch that moves the place where EditorClient::textFieldDidBeginEditing() gets called from RenderTextControl::subtreeHasChanged() to HTMLTextFieldInnerTextElement::defaultEventHandler() although I don't know if this is the best way? Would it be preferable that there is an EditorClient::textFieldMaybeBeginEditing method as well as textFieldDidBeginEditing which would get called when the control gets focused and textFieldDidBeginEditing would be still called where it is now? Or am I completely barking up the wrong tree here and completely missed something?
Attachments
Patch to call textFieldDidStartEditing whenever a text control receives focus (2.00 KB, patch)
2008-04-15 04:24 PDT, iain
darin: review-
Patch to call textFieldDidStartEditing whenever a text control receives focus (1.70 KB, patch)
2008-06-06 05:19 PDT, iain
no flags
iain
Comment 1 2008-04-15 04:24:10 PDT
Created attachment 20549 [details] Patch to call textFieldDidStartEditing whenever a text control receives focus
Alexey Proskuryakov
Comment 2 2008-04-15 08:08:24 PDT
Comment on attachment 20549 [details] Patch to call textFieldDidStartEditing whenever a text control receives focus Marking for review, so this doesn't get lost.
Adele Peterson
Comment 3 2008-04-27 22:31:56 PDT
CCing Justin. He was looking into this a while ago...
Darin Adler
Comment 4 2008-05-24 23:20:10 PDT
Comment on attachment 20549 [details] Patch to call textFieldDidStartEditing whenever a text control receives focus 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.
iain
Comment 5 2008-06-06 05:19:40 PDT
Created attachment 21524 [details] Patch to call textFieldDidStartEditing whenever a text control receives focus
iain
Comment 6 2008-06-06 05:23:15 PDT
> The indentation of the code looks wrong, and part of the patch is only this > indentation change on some existing code. Ah, yes, I had emacs in indent tab mode, but WebKit uses spaces to indent. > As with any WebKit bug fix, the patch needs to include a regression test that > demonstrates the problem. And a ChangeLog entry. The new patch includes a ChangeLog entry and fixes all your comments above. I'm not entirely sure where regression tests go, and to be honest I'm not sure how one would go about writing an automated test for what is more a change of behaviour rather than a bug per say. But maybe if I see what the other regression tests look like I'll have an epiphany.
iain
Comment 7 2008-06-09 07:56:06 PDT
I've just realised (well, alp told me) that what I'm looking for here is provided by the EditorClient::setInputMethodState() so I'm closing this as invalid. Apologies for not noticing it sooner.
Note You need to log in before you can comment on or make changes to this bug.