Bug 50050

Summary: [Qt] plugins/keyboard-events.html fails after r72717
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, abecsi, benjamin, commit-queue, eric, kling, mrobinson, ossy, robert, webkit.review.bot
Priority: P2 Keywords: LayoutTestFailure, Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Martin Robinson 2010-11-24 20:33:33 PST
Fixes to the Unix version of TestNetscapePlugin have uncovered a bug in PluginViewQt. This is related to a special-case for DumpRenderTree.
Comment 1 Martin Robinson 2010-11-24 20:35:48 PST
Committed r72719: <http://trac.webkit.org/changeset/72719>
Comment 2 WebKit Review Bot 2010-11-24 22:17:26 PST
http://trac.webkit.org/changeset/72719 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
editing/selection/extend-by-character-002.html
Comment 3 Robert Hogan 2011-01-16 10:04:31 PST
Created attachment 79103 [details]
Patch
Comment 4 Andreas Kling 2011-01-16 11:17:32 PST
Comment on attachment 79103 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79103&action=review

> Source/WebCore/plugins/qt/PluginViewQt.cpp:427
> +        const char* chr = qKeyEvent->text().left(1).toUtf8().constData();
> +        xEvent->xkey.keycode = XKeysymToKeycode(QX11Info::display(), XStringToKeysym(chr));

'chr' points to deleted data on the second line since the QByteArray returned by toUtf8() is temporary.
Comment 5 Robert Hogan 2011-01-16 11:59:53 PST
Created attachment 79106 [details]
Patch
Comment 6 Andreas Kling 2011-01-16 12:34:23 PST
Comment on attachment 79106 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79106&action=review

LGTM, thanks for attending to this!

> Source/WebCore/plugins/qt/PluginViewQt.cpp:426
> +        QString chr = qKeyEvent->text().left(1);

Nit: We don't abbreviate variable names in WebKit.
Comment 7 Robert Hogan 2011-01-16 12:48:34 PST
Created attachment 79108 [details]
Patch
Comment 8 WebKit Commit Bot 2011-01-16 13:08:43 PST
Comment on attachment 79108 [details]
Patch

Clearing flags on attachment: 79108

Committed r75894: <http://trac.webkit.org/changeset/75894>
Comment 9 WebKit Commit Bot 2011-01-16 13:08:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Robert Hogan 2011-01-16 14:09:19 PST
Committed r75895: <http://trac.webkit.org/changeset/75895>
Comment 11 WebKit Review Bot 2011-01-16 17:58:21 PST
http://trac.webkit.org/changeset/75897 might have broken Leopard Intel Release (Build)
Comment 12 Robert Hogan 2011-01-17 12:17:02 PST
I had to re-skip plugins/keyboard-events.html because it failed on the buildbot.

The change that broke this originally for Qt is http://trac.webkit.org/changeset/72717

I think the reason my patch failed is as follows, and am hoping Martin might be amenable to going back to using the keysym for this event.

In order to get the keycode of the keyboard event, we have to do:

xEvent->xkey.keycode = XKeysymToKeycode(QX11Info::display(), XStringToKeysym(keyText.toUtf8().constData()));

At first I thought it failed on the buildbot because it was headless, but the tests are run under xvfb so that was only half-right. The real reason, it seems to me, is because xvfb doesn't have a keyboard, so XKeysymToKeycode will always fail. (See http://lists.apple.com/archives/x11-users/2009/Apr/msg00016.html for an example of this elsewhere).

Since there's no keyboard mapping available to xvfb it can't determine the keycode. I recreated the problem in Ossy's 'QtWebKit Builder and Tester VM (Q-BAT VM)'.

I'm going to drop to a terminal session on this machine with no x server running so see if I can recreate it here too. I'll come back with my results but it looks as if using the keycodes won't work for us running our buildbot under xvfb.

Looking at the output from the other buildbots I think qt may be alone running the layout tests under xvfb. Who would be able to confirm that do you know?
Comment 13 Robert Hogan 2011-01-17 12:58:38 PST
So much for that theory - I can pass the test using xvfb-run in a session with no X server running on my own machine. The test always fails for me in the VM.

My version of Xvfb is from xorg-server 1.7.6, the version of Xvfb on the VM/buildbot is 1.4.2. Mmmm...

So that's my new copper-bottomed theory. I guess we wait for the buildbot to upgrade to debian squeeze and try again!
Comment 14 Csaba Osztrogon√°c 2011-01-17 13:02:43 PST
Bbandix, could you check Robert's idea on your hyper-up-to-date Arch Linux? :)
Comment 15 Robert Hogan 2011-01-17 13:31:53 PST
For ease of reference, this should pass:

xvfb-run --server-args="-screen 0 1024x768x24" Tools/Scripts/run-webkit-tests --
no-launch-safari --qt plugins/keyboard-events.html $@
Comment 16 Robert Hogan 2011-05-20 04:35:56 PDT
Committed r86938: <http://trac.webkit.org/changeset/86938>