Bug 27827

Summary: [Chromium] Functions Keys don't work in google spreadsheet.
Product: WebKit Reporter: Hironori Bono <hbono>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, estade, evan, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
A quick fix
eric: review-
The second quick fix
none
The third quick fix.
eric: review-
The fourth quick fix
eric: review+, eric: commit-queue-
The fifth quick fix none

Description Hironori Bono 2009-07-30 00:19:12 PDT
(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.
Comment 1 Hironori Bono 2009-07-31 04:27:15 PDT
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?
Comment 2 Eric Seidel (no email) 2009-07-31 15:16:40 PDT
Comment on attachment 33871 [details]
A quick fix

Woh woh.  Please end the copy-pasted insanity in DumpRenderTree.
Comment 3 Hironori Bono 2009-08-10 03:41:23 PDT
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,
Comment 4 Hironori Bono 2009-08-10 03:42:44 PDT
Created attachment 34444 [details]
The third quick fix.

Sorry, I attached a wrong patch.
Comment 5 Eric Seidel (no email) 2009-08-12 23:08:42 PDT
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?
Comment 6 Hironori Bono 2009-08-13 01:50:47 PDT
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.)
Comment 7 Eric Seidel (no email) 2009-08-19 00:06:50 PDT
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.
Comment 8 Eric Seidel (no email) 2009-08-19 00:20:13 PDT
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.
Comment 9 Eric Seidel (no email) 2009-08-19 00:33:08 PDT
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.
Comment 10 Evan Martin 2009-08-24 16:03:12 PDT
Ping.  Who needs to do something here?
Comment 11 Adam Barth 2009-08-24 16:30:16 PDT
Looks like either someone needs to update the patch so it applies cleanly to TOT or some need to land this manually.
Comment 12 Hironori Bono 2009-08-24 21:56:28 PDT
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.
Comment 13 Adam Barth 2009-08-24 23:06:01 PDT
Comment on attachment 38522 [details]
The fifth quick fix

Forwarding Eric's review+ to the updated patch.
Comment 14 Adam Barth 2009-08-24 23:53:40 PDT
Comment on attachment 38522 [details]
The fifth quick fix

Clearing flags on attachment: 38522

Committed r47741: <http://trac.webkit.org/changeset/47741>
Comment 15 Adam Barth 2009-08-24 23:53:44 PDT
All reviewed patches have been landed.  Closing bug.