Summary: | [Qt]REGRESSION(r82243): fast/events/onsearch-enter.html fails | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, ademar, commit-queue, eric, kling, max.hong.shen, menard, tonikitoo, webkit.review.bot | ||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | All | ||||||
Bug Depends on: | 58144 | ||||||
Bug Blocks: | 33179 | ||||||
Attachments: |
|
Description
Csaba Osztrogonác
2011-03-30 10:27:58 PDT
I added it to the skipped list: http://trac.webkit.org/changeset/82476 Created attachment 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.
Comment on attachment 88701 [details] first try Clearing flags on attachment: 88701 Committed r83295: <http://trac.webkit.org/changeset/83295> All reviewed patches have been landed. Closing bug. 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 (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. (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. 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. (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. (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. (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). (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 :-) Close this bug which is fixed in 33179 |