18510 2008-04-15 04:22:44 -0700 Text controls only emit the start editing signal after the first keypress 2008-06-09 07:56:06 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) PC Linux RESOLVED INVALID P2 Normal --- 1 iain webkit-unassigned adele justin.garcia oldest_to_newest 77677 0 iain 2008-04-15 04:22:44 -0700 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? 77678 1 20549 iain 2008-04-15 04:24:10 -0700 Created attachment 20549 Patch to call textFieldDidStartEditing whenever a text control receives focus 77688 2 20549 ap 2008-04-15 08:08:24 -0700 Comment on attachment 20549 Patch to call textFieldDidStartEditing whenever a text control receives focus Marking for review, so this doesn't get lost. 79083 3 adele 2008-04-27 22:31:56 -0700 CCing Justin. He was looking into this a while ago... 81413 4 20549 darin 2008-05-24 23:20:10 -0700 Comment on attachment 20549 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. 82418 5 21524 iain 2008-06-06 05:19:40 -0700 Created attachment 21524 Patch to call textFieldDidStartEditing whenever a text control receives focus 82419 6 iain 2008-06-06 05:23:15 -0700 > 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. 82750 7 iain 2008-06-09 07:56:06 -0700 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. 20549 2008-04-15 04:24:10 -0700 2008-06-06 05:19:40 -0700 Patch to call textFieldDidStartEditing whenever a text control receives focus start-editing.patch text/plain 2047 iain ZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9IVE1MVGV4dEZpZWxkSW5uZXJFbGVtZW50LmNwcCBi L1dlYkNvcmUvaHRtbC9IVE1MVGV4dEZpZWxkSW5uZXJFbGVtZW50LmNwcAppbmRleCBlNmQ1ODcx Li41NTQ1ZTI5IDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvSFRNTFRleHRGaWVsZElubmVyRWxl bWVudC5jcHAKKysrIGIvV2ViQ29yZS9odG1sL0hUTUxUZXh0RmllbGRJbm5lckVsZW1lbnQuY3Bw CkBAIC01NywxMCArNTcsMTYgQEAgdm9pZCBIVE1MVGV4dEZpZWxkSW5uZXJUZXh0RWxlbWVudDo6 ZGVmYXVsdEV2ZW50SGFuZGxlcihFdmVudCogZXZ0KQogICAgIGlmIChzaGFkb3dBbmNlc3RvciAm JiBzaGFkb3dBbmNlc3Rvci0+cmVuZGVyZXIoKSkgewogICAgICAgICBBU1NFUlQoc2hhZG93QW5j ZXN0b3ItPnJlbmRlcmVyKCktPmlzVGV4dEZpZWxkKCkgfHwgc2hhZG93QW5jZXN0b3ItPnJlbmRl cmVyKCktPmlzVGV4dEFyZWEoKSk7CiAgICAgICAgIGlmIChldnQtPmlzQmVmb3JlVGV4dEluc2Vy dGVkRXZlbnQoKSkKLSAgICAgICAgICAgIGlmIChzaGFkb3dBbmNlc3Rvci0+cmVuZGVyZXIoKS0+ aXNUZXh0RmllbGQoKSkKKwkgICAgaWYgKHNoYWRvd0FuY2VzdG9yLT5yZW5kZXJlcigpLT5pc1Rl eHRGaWVsZCgpKQogICAgICAgICAgICAgICAgIHN0YXRpY19jYXN0PEhUTUxJbnB1dEVsZW1lbnQq PihzaGFkb3dBbmNlc3RvciktPmRlZmF1bHRFdmVudEhhbmRsZXIoZXZ0KTsKLSAgICAgICAgICAg IGVsc2UKLSAgICAgICAgICAgICAgICBzdGF0aWNfY2FzdDxIVE1MVGV4dEFyZWFFbGVtZW50Kj4o c2hhZG93QW5jZXN0b3IpLT5kZWZhdWx0RXZlbnRIYW5kbGVyKGV2dCk7CisJICAgIGVsc2UKKwkJ c3RhdGljX2Nhc3Q8SFRNTFRleHRBcmVhRWxlbWVudCo+KHNoYWRvd0FuY2VzdG9yKS0+ZGVmYXVs dEV2ZW50SGFuZGxlcihldnQpOworCQorCWlmIChldnQtPnR5cGUoKSA9PSBjbGlja0V2ZW50KSB7 CisJICAgIEhUTUxJbnB1dEVsZW1lbnQqIGlucHV0ID0gc3RhdGljX2Nhc3Q8SFRNTElucHV0RWxl bWVudCo+KHNoYWRvd0FuY2VzdG9yKTsKKwkgICAgc3RhdGljX2Nhc3Q8UmVuZGVyVGV4dENvbnRy b2wqPihzaGFkb3dBbmNlc3Rvci0+cmVuZGVyZXIoKSktPmRvY3VtZW50KCktPmZyYW1lKCktPnRl eHRGaWVsZERpZEJlZ2luRWRpdGluZyhpbnB1dCk7CisJfQorCQogICAgICAgICBpZiAoZXZ0LT50 eXBlKCkgPT0gd2Via2l0RWRpdGFibGVDb250ZW50Q2hhbmdlZEV2ZW50KQogICAgICAgICAgICAg c3RhdGljX2Nhc3Q8UmVuZGVyVGV4dENvbnRyb2wqPihzaGFkb3dBbmNlc3Rvci0+cmVuZGVyZXIo KSktPnN1YnRyZWVIYXNDaGFuZ2VkKCk7CiAgICAgfQpkaWZmIC0tZ2l0IGEvV2ViQ29yZS9yZW5k ZXJpbmcvUmVuZGVyVGV4dENvbnRyb2wuY3BwIGIvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGV4 dENvbnRyb2wuY3BwCmluZGV4IDI1MTk3YWQuLjg1ZDU4YjEgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUv cmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmNwcAorKysgYi9XZWJDb3JlL3JlbmRlcmluZy9S ZW5kZXJUZXh0Q29udHJvbC5jcHAKQEAgLTU0MiwxMSArNTQyLDYgQEAgdm9pZCBSZW5kZXJUZXh0 Q29udHJvbDo6c3VidHJlZUhhc0NoYW5nZWQoKQogICAgICAgICBpZiAoIWlucHV0LT5nZXRBdHRy aWJ1dGUoaW5jcmVtZW50YWxBdHRyKS5pc051bGwoKSkKICAgICAgICAgICAgIHN0YXJ0U2VhcmNo RXZlbnRUaW1lcigpOwogCi0gICAgICAgIGlmICghd2FzRGlydHkpIAotICAgICAgICAgICAgaWYg KGlucHV0LT5mb2N1c2VkKCkpCi0gICAgICAgICAgICAgICAgaWYgKEZyYW1lKiBmcmFtZSA9IGRv Y3VtZW50KCktPmZyYW1lKCkpCi0gICAgICAgICAgICAgICAgICAgIGZyYW1lLT50ZXh0RmllbGRE aWRCZWdpbkVkaXRpbmcoaW5wdXQpOwotCiAgICAgICAgIGlmIChpbnB1dC0+Zm9jdXNlZCgpKQog ICAgICAgICAgICAgaWYgKEZyYW1lKiBmcmFtZSA9IGRvY3VtZW50KCktPmZyYW1lKCkpCiAgICAg ICAgICAgICAgICAgZnJhbWUtPnRleHREaWRDaGFuZ2VJblRleHRGaWVsZChpbnB1dCk7Cg== 21524 2008-06-06 05:19:40 -0700 2008-06-06 05:19:40 -0700 Patch to call textFieldDidStartEditing whenever a text control receives focus 18510-tk2.patch text/plain 1736 iain ZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9IVE1MVGV4dEZpZWxkSW5uZXJFbGVtZW50LmNwcCBi L1dlYkNvcmUvaHRtbC9IVE1MVGV4dEZpZWxkSW5uZXJFbGVtZW50LmNwcAppbmRleCBlNmQ1ODcx Li5kMTVkMThhIDEwMDY0NAotLS0gYS9XZWJDb3JlL2h0bWwvSFRNTFRleHRGaWVsZElubmVyRWxl bWVudC5jcHAKKysrIGIvV2ViQ29yZS9odG1sL0hUTUxUZXh0RmllbGRJbm5lckVsZW1lbnQuY3Bw CkBAIC02MSw2ICs2MSwxNiBAQCB2b2lkIEhUTUxUZXh0RmllbGRJbm5lclRleHRFbGVtZW50Ojpk ZWZhdWx0RXZlbnRIYW5kbGVyKEV2ZW50KiBldnQpCiAgICAgICAgICAgICAgICAgc3RhdGljX2Nh c3Q8SFRNTElucHV0RWxlbWVudCo+KHNoYWRvd0FuY2VzdG9yKS0+ZGVmYXVsdEV2ZW50SGFuZGxl cihldnQpOwogICAgICAgICAgICAgZWxzZQogICAgICAgICAgICAgICAgIHN0YXRpY19jYXN0PEhU TUxUZXh0QXJlYUVsZW1lbnQqPihzaGFkb3dBbmNlc3RvciktPmRlZmF1bHRFdmVudEhhbmRsZXIo ZXZ0KTsKKworICAgICAgICBpZiAoZXZ0LT50eXBlKCkgPT0gY2xpY2tFdmVudCkgeworICAgICAg ICAgICAgRWxlbWVudCogZWxlbWVudCA9IHN0YXRpY19jYXN0PEVsZW1lbnQqPihzaGFkb3dBbmNl c3Rvcik7CisgICAgICAgICAgICBGcmFtZSogZnJhbWU7CisKKyAgICAgICAgICAgIGZyYW1lID0g c3RhdGljX2Nhc3Q8UmVuZGVyT2JqZWN0Kj4oc2hhZG93QW5jZXN0b3ItPnJlbmRlcmVyKCkpLT5k b2N1bWVudCgpLT5mcmFtZSgpOworICAgICAgICAgICAgaWYgKGZyYW1lKQorICAgICAgICAgICAg ICAgIGZyYW1lLT50ZXh0RmllbGREaWRCZWdpbkVkaXRpbmcoZWxlbWVudCk7CisgICAgICAgIH0K KwogICAgICAgICBpZiAoZXZ0LT50eXBlKCkgPT0gd2Via2l0RWRpdGFibGVDb250ZW50Q2hhbmdl ZEV2ZW50KQogICAgICAgICAgICAgc3RhdGljX2Nhc3Q8UmVuZGVyVGV4dENvbnRyb2wqPihzaGFk b3dBbmNlc3Rvci0+cmVuZGVyZXIoKSktPnN1YnRyZWVIYXNDaGFuZ2VkKCk7CiAgICAgfQpkaWZm IC0tZ2l0IGEvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGV4dENvbnRyb2wuY3BwIGIvV2ViQ29y ZS9yZW5kZXJpbmcvUmVuZGVyVGV4dENvbnRyb2wuY3BwCmluZGV4IGE2NjBjOWEuLjFhZDMwNjgg MTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclRleHRDb250cm9sLmNwcAorKysg Yi9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUZXh0Q29udHJvbC5jcHAKQEAgLTU0MiwxMSArNTQy LDYgQEAgdm9pZCBSZW5kZXJUZXh0Q29udHJvbDo6c3VidHJlZUhhc0NoYW5nZWQoKQogICAgICAg ICBpZiAoIWlucHV0LT5nZXRBdHRyaWJ1dGUoaW5jcmVtZW50YWxBdHRyKS5pc051bGwoKSkKICAg ICAgICAgICAgIHN0YXJ0U2VhcmNoRXZlbnRUaW1lcigpOwogCi0gICAgICAgIGlmICghd2FzRGly dHkpIHsKLSAgICAgICAgICAgIGlmIChpbnB1dC0+Zm9jdXNlZCgpKQotICAgICAgICAgICAgICAg IGlmIChGcmFtZSogZnJhbWUgPSBkb2N1bWVudCgpLT5mcmFtZSgpKQotICAgICAgICAgICAgICAg ICAgICBmcmFtZS0+dGV4dEZpZWxkRGlkQmVnaW5FZGl0aW5nKGlucHV0KTsKLSAgICAgICAgfQog ICAgICAgICBpZiAoaW5wdXQtPmZvY3VzZWQoKSkKICAgICAgICAgICAgIGlmIChGcmFtZSogZnJh bWUgPSBkb2N1bWVudCgpLT5mcmFtZSgpKQogICAgICAgICAgICAgICAgIGZyYW1lLT50ZXh0RGlk Q2hhbmdlSW5UZXh0RmllbGQoaW5wdXQpOwo=