Bug 27827 – [Chromium] Functions Keys don't work in google spreadsheet.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Hironori Bono]

(Copied from <http://crbug.com/14821>.)

Chrome Version : r22038
OS + version : Ubuntu 8.04
CPU architecture (32-bit / 64-bit): 64
window manager : gnome
URLs (if applicable) : <http://docs.google.com/>
Behavior in Firefox 3.x (if applicable):
Behavior in Chrome for Windows (optional):

What steps will reproduce the problem?
1. open spreadsheet em doc.google.com
2. enter data in cell
3. return to cell and press f2 to edit
4. previously entered data disappears and cell is empty.

What is the expected result?

What happens instead?

Please provide any additional information below. Attach a screenshot
and backtrace if possible.

WebCore::windowsKeyCodeForKeyEvent() (in "WebCore/platform/chromium/KeyCodeConversionGtk.cpp") doesn't have mappings for GDK Function keys (i.e. GDK_F1...,GDK_F24) and cannot send function-key events to a web page. We need to copy the mappings from "WebCore/platform/gtk/KeyEventGdk.cpp".

Also, it may be good to change "WebKitTools/DumpRenderTree/mac/EventSendingController.mm" so that we can send a function-key event through a eventSender.keyDown("F1") call and write layout tests for this issue.

[Hironori Bono]

Created attachment 33871 [details]
A quick fix

The attached layout test fails on platforms whose eventSender.keyDown() cannot send function-key events.
I'm wondering which is the better option: moving this layout test to "LayoutTest/platform" or changing "Skipped" files?

[Eric Seidel (no email)]

Comment on attachment 33871 [details]
A quick fix

Woh woh.  Please end the copy-pasted insanity in DumpRenderTree.

[Hironori Bono]

Created attachment 34443 [details]
The second quick fix

Thank you for your review and sorry for my slow update.
I updated EventSendingController.mm to avoid copy-and-pastes. Also, this change moves the new layout test to "platform/mac" because I don't have any development environment of WebKit except Mac, i.e. I can verify this test works only on Mac (Leopard).

Regards,

[Hironori Bono]

Created attachment 34444 [details]
The third quick fix.

Sorry, I attached a wrong patch.

[Eric Seidel (no email)]

Comment on attachment 34444 [details]
The third quick fix.

Looks fine.  Except the results should not be in platform/mac, but rather right next to the test.  At least I don't think this test is platform specific, is it?

[Hironori Bono]

Created attachment 34721 [details]
The fourth quick fix

Thank you for your review and comments.
I have moved the new layout test "keydown-function-keys.html" from "LayoutTests/platform/mac/fast/event" to "LayoutTests/fast/event". Also, I added this test to "LayoutTests/platform/{gtk,qt,win}/Skipped" files because eventSender.keyDown() cannot send function-key events on the platforms. (I don't have any idea how to support them since I don't have build environments for these platforms.)

[Eric Seidel (no email)]

Comment on attachment 34721 [details]
The fourth quick fix

I would have just inlined the "runScript()" stuff in the second script tag. :)  But this looks totally fine as is.

[Eric Seidel (no email)]

Comment on attachment 34721 [details]
The fourth quick fix

Rejecting patch 34721 from commit-queue.  This patch will require manual commit.

Patch https://bugs.webkit.org/attachment.cgi?id=34721 from bug 27827 failed to download and apply.

[Eric Seidel (no email)]

patching file LayoutTests/platform/gtk/Skipped
Hunk #1 FAILED at 5979.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/Skipped.rej
patch -p0 "LayoutTests/platform/gtk/Skipped" returned 1.  Pass --force to ignore patch failures.

[Evan Martin]

Ping.  Who needs to do something here?

[Adam Barth]

Looks like either someone needs to update the patch so it applies cleanly to TOT or some need to land this manually.

[Hironori Bono]

Created attachment 38522 [details]
The fifth quick fix

I have updated this change to fix a conflict in "LayoutTests/platform/gtk/Skipped" and move the code in runScript() into the second <script> element.
I wish this fix can be landed without conflicts.

[Adam Barth]

Comment on attachment 38522 [details]
The fifth quick fix

Forwarding Eric's review+ to the updated patch.

[Adam Barth]

Comment on attachment 38522 [details]
The fifth quick fix

Clearing flags on attachment: 38522

Committed r47741: <http://trac.webkit.org/changeset/47741>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.