Bug 57472 – [Qt]REGRESSION(r82243): fast/events/onsearch-enter.html fails

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

I added it to the skipped list: http://trac.webkit.org/changeset/82476

[Yi Shen]

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.

[WebKit Commit Bot]

Comment on attachment 88701 [details]
first try

Clearing flags on attachment: 88701

Committed r83295: <http://trac.webkit.org/changeset/83295>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

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

[Antonio Gomes]

(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.

[Yi Shen]

(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.

[Yi Shen]

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.

[Antonio Gomes]

(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.

[Yi Shen]

(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.

[Ademar Reis]

(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).

[Ademar Reis]

(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 :-)

[Yi Shen]

Close this bug which is fixed in 33179