57472 2011-03-30 10:27:58 -0700 [Qt]REGRESSION(r82243): fast/events/onsearch-enter.html fails 2011-05-19 14:35:31 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC All RESOLVED FIXED Qt, QtTriaged P2 Normal --- 58144 33179 1 ossy webkit-unassigned abarth ademar commit-queue eric kling max.hong.shen menard tonikitoo webkit.review.bot oldest_to_newest 376442 0 ossy 2011-03-30 10:27:58 -0700 It seems the event fires twice: --- /home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/layout-test-results/fast/events /onsearch-enter-expected.txt 2011-03-29 10:26:35.556752666 -0700 +++ /home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/layout-test-results/fast/events/onsearch-enter-actual.txt 2011-03-29 10:26:35.556752666 -0700 @@ -1,4 +1,5 @@ This tests that onSearch fires correctly. Test Passed +Test Passed 376499 1 ossy 2011-03-30 11:23:51 -0700 I added it to the skipped list: http://trac.webkit.org/changeset/82476 381718 2 88701 max.hong.shen 2011-04-07 14:36:47 -0700 Created attachment 88701 first try The problem is that EditorClientQt::handleInputMethodKeydown() is invoked by both keydown event and keypress event, and inserts two new lines (and sends onsearch events twice in this bug). One simple solution is by checking defaultPrevented() for each event before trigger the insertnewline action. 382247 3 88701 commit-queue 2011-04-08 08:24:28 -0700 Comment on attachment 88701 first try Clearing flags on attachment: 88701 Committed r83295: <http://trac.webkit.org/changeset/83295> 382248 4 commit-queue 2011-04-08 08:24:33 -0700 All reviewed patches have been landed. Closing bug. 382280 5 webkit.review.bot 2011-04-08 09:18:53 -0700 http://trac.webkit.org/changeset/83295 might have broken Qt Linux Release The following tests are not passing: fast/forms/button-enter-click.html fast/forms/enter-clicks-buttons.html fast/forms/implicit-submission.html fast/forms/select-double-onchange.html 382303 6 tonikitoo 2011-04-08 09:46:52 -0700 (In reply to comment #5) > http://trac.webkit.org/changeset/83295 might have broken Qt Linux Release > The following tests are not passing: > fast/forms/button-enter-click.html > fast/forms/enter-clicks-buttons.html > fast/forms/implicit-submission.html > fast/forms/select-double-onchange.html Rolling it out for offline fix. 382382 7 max.hong.shen 2011-04-08 10:54:38 -0700 (In reply to comment #6) > (In reply to comment #5) > > http://trac.webkit.org/changeset/83295 might have broken Qt Linux Release > > The following tests are not passing: > > fast/forms/button-enter-click.html > > fast/forms/enter-clicks-buttons.html > > fast/forms/implicit-submission.html > > fast/forms/select-double-onchange.html > > Rolling it out for offline fix. Sorry for it. I am looking at the issues. 382880 8 max.hong.shen 2011-04-10 07:32:33 -0700 This regression is caused by the patch launched in https://bugs.webkit.org/show_bug.cgi?id=33179, and I doubt that patch is correct. First, I saw a problem in the unit test for that patch, in which it tries to insert a new line by sending a enter key event QKeyEvent keyEnter(QEvent::KeyPress, Qt::Key_Enter, Qt::NoModifier); page->event(&keyEnter); This is incorrect in Webkit, in order to adding a new line, we should add a text for the key event as below, QKeyEvent keyEnter(QEvent::KeyPress, Qt::Key_Enter, Qt::NoModifier, "\n"); page->event(&keyEnter); Secondly, the issue in bug 33179 is a symbian platform only, which can't be reproduced on Linux. While, the patch impacts all the platforms and causes this regression. I will take a look at the bug 33179 on symbian to see whether there is another way to fix it. 382889 9 tonikitoo 2011-04-10 09:00:51 -0700 (In reply to comment #8) > This regression is caused by the patch launched in https://bugs.webkit.org/show_bug.cgi?id=33179, and I doubt that patch is correct. > > First, I saw a problem in the unit test for that patch, in which it tries to insert a new line by sending a enter key event > > QKeyEvent keyEnter(QEvent::KeyPress, Qt::Key_Enter, Qt::NoModifier); > page->event(&keyEnter); > > This is incorrect in Webkit, in order to adding a new line, we should add a text for the key event as below, > > QKeyEvent keyEnter(QEvent::KeyPress, Qt::Key_Enter, Qt::NoModifier, "\n"); > page->event(&keyEnter); > > Secondly, the issue in bug 33179 is a symbian platform only, which can't be reproduced on Linux. While, the patch impacts all the platforms and causes this regression. > > I will take a look at the bug 33179 on symbian to see whether there is another way to fix it. please comment in that bug, if you have not yet. It sounds really bad. 383159 10 max.hong.shen 2011-04-11 07:34:55 -0700 (In reply to comment #2) > Created an attachment (id=88701) [details] > first try > > The problem is that EditorClientQt::handleInputMethodKeydown() is invoked by both keydown event and keypress event, and inserts two new lines (and sends onsearch events twice in this bug). One simple solution is by checking defaultPrevented() for each event before trigger the insertnewline action. Please ignore the comment above, which is not completely right. I will put some comment in https://bugs.webkit.org/show_bug.cgi?id=33179, which is the one that caused this regression. 406599 11 ademar 2011-05-19 13:18:20 -0700 (In reply to comment #10) > Please ignore the comment above, which is not completely right. I will put some comment in https://bugs.webkit.org/show_bug.cgi?id=33179, which is the one that caused this regression. Should this bug still be open then? (Bug 33179, which depends on this one, is closed already). 406600 12 ademar 2011-05-19 13:19:18 -0700 (In reply to comment #11) > (In reply to comment #10) > > Please ignore the comment above, which is not completely right. I will put some comment in https://bugs.webkit.org/show_bug.cgi?id=33179, which is the one that caused this regression. > > Should this bug still be open then? (Bug 33179, which depends on this one, is closed already). Duh, actually the bug is still open, but all patches have been landed. /me lost :-) 406656 13 max.hong.shen 2011-05-19 14:35:31 -0700 Close this bug which is fixed in 33179 88701 2011-04-07 14:36:47 -0700 2011-04-08 08:24:28 -0700 first try 57472.patch text/plain 2931 max.hong.shen SW5kZXg6IFNvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJLaXQvcXQvQ2hhbmdlTG9nCShyZXZpc2lvbiA4MzIwOCkKKysrIFNvdXJjZS9XZWJLaXQvcXQv Q2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDQtMDcgIFlp IFNoZW4gIDx5aS40LnNoZW5Abm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFtRdF1SRUdSRVNTSU9OKHI4MjI0Myk6IGZhc3QvZXZlbnRz L29uc2VhcmNoLWVudGVyLmh0bWwgZmFpbHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTU3NDcyCisKKyAgICAgICAgQXZvaWQgdG8gaW5zZXJ0IG5ldyBs aW5lIGZvciBib3RoIGtleWRvd24gZXZlbnQgJiBrZXlwcmVzcyBldmVudC4KKworICAgICAgICAq IFdlYkNvcmVTdXBwb3J0L0VkaXRvckNsaWVudFF0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkVk aXRvckNsaWVudFF0OjpoYW5kbGVJbnB1dE1ldGhvZEtleWRvd24pOgorCiAyMDExLTA0LTA3ICBD YWlvIE1hcmNlbG8gZGUgT2xpdmVpcmEgRmlsaG8gIDxjYWlvLm9saXZlaXJhQG9wZW5ib3NzYS5v cmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQmVuamFtaW4gUG91bGFpbi4KSW5kZXg6IFNvdXJj ZS9XZWJLaXQvcXQvV2ViQ29yZVN1cHBvcnQvRWRpdG9yQ2xpZW50UXQuY3BwCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9XZWJLaXQvcXQvV2ViQ29yZVN1cHBvcnQvRWRpdG9yQ2xpZW50UXQuY3BwCShy ZXZpc2lvbiA4MzE3MykKKysrIFNvdXJjZS9XZWJLaXQvcXQvV2ViQ29yZVN1cHBvcnQvRWRpdG9y Q2xpZW50UXQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01MzQsMTIgKzUzNCwxMyBAQCB2b2lkIEVk aXRvckNsaWVudFF0OjpoYW5kbGVJbnB1dE1ldGhvZEtlCiB7CiAjaWZuZGVmIFFUX05PX1NIT1JU Q1VUCiAgICAgY29uc3QgUGxhdGZvcm1LZXlib2FyZEV2ZW50KiBrZXZlbnQgPSBldmVudC0+a2V5 RXZlbnQoKTsKLSAgICBpZiAoa2V2ZW50LT50eXBlKCkgPT0gUGxhdGZvcm1LZXlib2FyZEV2ZW50 OjpSYXdLZXlEb3duKSB7CisgICAgaWYgKGtldmVudC0+dHlwZSgpID09IFBsYXRmb3JtS2V5Ym9h cmRFdmVudDo6UmF3S2V5RG93biAmJiAhZXZlbnQtPmRlZmF1bHRQcmV2ZW50ZWQoKSkgewogICAg ICAgICBRV2ViUGFnZTo6V2ViQWN0aW9uIGFjdGlvbiA9IFFXZWJQYWdlUHJpdmF0ZTo6ZWRpdG9y QWN0aW9uRm9yS2V5RXZlbnQoa2V2ZW50LT5xdEV2ZW50KCkpOwogICAgICAgICBzd2l0Y2ggKGFj dGlvbikgewogICAgICAgICBjYXNlIFFXZWJQYWdlOjpJbnNlcnRQYXJhZ3JhcGhTZXBhcmF0b3I6 CiAgICAgICAgIGNhc2UgUVdlYlBhZ2U6Okluc2VydExpbmVTZXBhcmF0b3I6CiAgICAgICAgICAg ICBtX3BhZ2UtPnRyaWdnZXJBY3Rpb24oYWN0aW9uKTsKKyAgICAgICAgICAgIGV2ZW50LT5zZXRE ZWZhdWx0SGFuZGxlZCgpOwogICAgICAgICAgICAgYnJlYWs7CiAgICAgICAgIGRlZmF1bHQ6CiAg ICAgICAgICAgICBicmVhazsKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzMjA4KQorKysgTGF5b3V0VGVz dHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTEtMDQtMDcg IFlpIFNoZW4gIDx5aS40LnNoZW5Abm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF1SRUdSRVNTSU9OKHI4MjI0Myk6IGZhc3QvZXZl bnRzL29uc2VhcmNoLWVudGVyLmh0bWwgZmFpbHMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTU3NDcyCisKKyAgICAgICAgVW5za2lwcyB0aGUgZmFzdC9l dmVudHMvb25zZWFyY2gtZW50ZXIuaHRtbCB0ZXN0LgorCisgICAgICAgICogcGxhdGZvcm0vcXQv U2tpcHBlZDoKKwogMjAxMS0wNC0wNyAgQnJpYW4gV2VpbnN0ZWluICA8YndlaW5zdGVpbkBhcHBs ZS5jb20+CiAKICAgICAgICAgRml4IGEgdHlwbyBpbiB0aGUgU2tpcHBlZCBsaXN0IGZyb20gcjgz MjAzLgpJbmRleDogTGF5b3V0VGVzdHMvcGxhdGZvcm0vcXQvU2tpcHBlZAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9xdC9Ta2lwcGVkCShyZXZpc2lvbiA4MzE3MykKKysrIExh eW91dFRlc3RzL3BsYXRmb3JtL3F0L1NraXBwZWQJKHdvcmtpbmcgY29weSkKQEAgLTMyMjcsMTAg KzMyMjcsNiBAQCBwbHVnaW5zL2VtYmVkLXByZWZlcnMtcGx1Z2lucy1mb3ItaW1hZ2VzCiAjIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NzMwNgogaHR0cC90ZXN0cy91 cmkvdXNlcm5hbWUtd2l0aC1uby1ob3N0bmFtZS5odG1sCiAKLSMgW1F0XVJFR1JFU1NJT04ocjgy MjQzKTogZmFzdC9ldmVudHMvb25zZWFyY2gtZW50ZXIuaHRtbCBmYWlscwotIyBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc0NzIKLWZhc3QvZXZlbnRzL29uc2VhcmNo LWVudGVyLmh0bWwKLQogIyBbUXRdIG1lZGlhL3ZpZGVvLXBsYXliYWNrcmF0ZS5odG1sIGZhaWxz CiAjIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NzQ3NgogbWVkaWEv dmlkZW8tcGxheWJhY2tyYXRlLmh0bWwK