Bug 44613 - [GTK] Provide Keyboard Events to Windowless plugins
Summary: [GTK] Provide Keyboard Events to Windowless plugins
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL: http://www.communitymx.com/content/so...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-25 09:06 PDT by Bharathwaaj
Modified: 2010-11-22 15:54 PST (History)
8 users (show)

See Also:


Attachments
This patch focuses the plugin element when mouse clicked on plugin. Also while handling keyboard event, the XEvent is first initialized by calling initXEvent. (2.18 KB, patch)
2010-08-25 09:17 PDT, Bharathwaaj
no flags Details | Formatted Diff | Diff
Updated based on Xan's comment (1.55 KB, patch)
2010-08-27 09:31 PDT, Bharathwaaj
no flags Details | Formatted Diff | Diff
pluginkeyboard.diff (5.42 KB, patch)
2010-09-05 01:52 PDT, Xan Lopez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bharathwaaj 2010-08-25 09:06:30 PDT
Keyboard events are not being provided to plugins in windowless mode. This is observed from the code as PluginView::focusPluginElement() is not being called during handleMouseEvent. Patch coming soon.
SVN TOT Revision: 61539
Comment 1 Bharathwaaj 2010-08-25 09:17:54 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.
Comment 2 Xan Lopez 2010-08-27 01:32:25 PDT
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.
Comment 3 Bharathwaaj 2010-08-27 09:17:41 PDT
(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
Comment 4 Bharathwaaj 2010-08-27 09:31:18 PDT
Created attachment 65723 [details]
Updated based on Xan's comment
Comment 5 Xan Lopez 2010-08-27 09:44:57 PDT
Comment on attachment 65723 [details]
Updated based on Xan's comment

LGTM.
Comment 6 WebKit Commit Bot 2010-08-28 00:01:06 PDT
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
Comment 7 Xan Lopez 2010-09-03 20:41:53 PDT
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?
Comment 8 Eric Seidel (no email) 2010-09-03 20:45:20 PDT
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. :)
Comment 9 Xan Lopez 2010-09-05 01:52:11 PDT
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.
Comment 10 Bharathwaaj 2010-09-06 03:36:26 PDT
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 11 Martin Robinson 2010-09-06 07:27:54 PDT
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).
Comment 12 Xan Lopez 2010-09-06 07:39:41 PDT
The mouse events are related to this, the other tests aren't, so I'll add those.
Comment 13 Xan Lopez 2010-09-06 07:40:05 PDT
Comment on attachment 66595 [details]
pluginkeyboard.diff

Landed as r66827
Comment 14 Xan Lopez 2010-09-06 07:40:32 PDT
Closing.
Comment 15 WebKit Review Bot 2010-09-06 08:17:09 PDT
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
Comment 16 Nicolas Dufresne 2010-11-22 15:54:24 PST
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.