| Summary: | [GTK] Provide Keyboard Events to Windowless plugins | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Bharathwaaj <bharathwaaj.s> |
| Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | abarth, bharathwaaj.s, commit-queue, eric, gustavo, nicolas, webkit.review.bot, xan.lopez |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| URL: | http://www.communitymx.com/content/source/E5141/wmodetrans.htm | ||
| Attachments: | |||
|
Description
Bharathwaaj
2010-08-25 09:06:30 PDT
Created attachment 65427 [details]
This patch focuses the plugin element when mouse clicked on plugin. Also while handling keyboard event, the XEvent is first initialized by calling initXEvent.
Please find the initial patch to fix. This is my first time submitting. So please forgive if there are any mistakes in the attachment.
The patch looks good, but would it be too much to ask to submit one patch per issue? They are not really related even though both are needed. Also, the formatting in the ChangeLog is a bit odd, the lines are too long. If you use emacs just press Alt-q to fix it. (In reply to comment #2) > The patch looks good, but would it be too much to ask to submit one patch per issue? They are not really related even though both are needed. Also, the formatting in the ChangeLog is a bit odd, the lines are too long. If you use emacs just press Alt-q to fix it. Thanks for the comment. I've created a new bug for initializing keyboard event. https://bugs.webkit.org/show_bug.cgi?id=44771 Created attachment 65723 [details]
Updated based on Xan's comment
Comment on attachment 65723 [details]
Updated based on Xan's comment
LGTM.
Comment on attachment 65723 [details] Updated based on Xan's comment Rejecting patch 65723 from commit-queue. Failed to run "['WebKitTools/Scripts/test-webkitpy']" exit_code: 1 Last 500 characters of output: tForThreadsToFinishTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py", line 291, in test_exception self.assertRaises(ValueError, self.run_one_thread, 'Exception') AssertionError: ValueError not raised ---------------------------------------------------------------------- Ran 524 tests in 22.235s FAILED (failures=1) Full output: http://queues.webkit.org/results/3865044 There's a test for this functionality in plugins/keyboard-events.html. Even with your patch attached I still get two kinds of failures there (the focus event is not received, and the keycodes are all wrong). Could you confirm this? Great! Lets file a bug about the failure and possibly unskip the test with gtk-specific results. :) glad to know we already have testing. We just need to turn it on for gtk eventually. :) Created attachment 66595 [details]
pluginkeyboard.diff
OK, took some time to try to make this pass the test. Two further fixes where needed, patch attached.
The new patch provided by Xan also fixes the following LayoutTests and needs to be removed from platform/gtk/Skipped file. plugins/mouse-events.html plugins/mouse-events-fixedpos.html plugins/geturlnotify-during-document-teardown.html plugins/private-browsing-mode.html Comment on attachment 66595 [details]
pluginkeyboard.diff
LGTM! Please unskip the other three tests when landing your patch as well (if they are indeed passing for you as well).
The mouse events are related to this, the other tests aren't, so I'll add those. Comment on attachment 66595 [details] pluginkeyboard.diff Landed as r66827 Closing. http://trac.webkit.org/changeset/66827 might have broken Qt Linux Release The following changes are on the blame list: http://trac.webkit.org/changeset/66827 http://trac.webkit.org/changeset/66828 http://trac.webkit.org/changeset/66829 Just found that 66827, sending a GdkEvent::keyval as if it was an valid XEvent::keycode is wrong. The original code was right, the XEvent::keycode matches the GdkEvent::hardware_keycode. Please have a look at the following bug for a fix: https://bugs.webkit.org/show_bug.cgi?id=49927 Obviously this is not covered by any test, so I might have to look into that. |