Bug 44613 – [GTK] Provide Keyboard Events to Windowless plugins

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Bharathwaaj]

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

[Bharathwaaj]

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.

[Xan Lopez]

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.

[Bharathwaaj]

(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

[Bharathwaaj]

Created attachment 65723 [details]
Updated based on Xan's comment

[Xan Lopez]

Comment on attachment 65723 [details]
Updated based on Xan's comment

LGTM.

[WebKit Commit Bot]

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

[Xan Lopez]

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?

[Eric Seidel (no email)]

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. :)

[Xan Lopez]

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.

[Bharathwaaj]

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

[Martin Robinson]

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).

[Xan Lopez]

The mouse events are related to this, the other tests aren't, so I'll add those.

[Xan Lopez]

Comment on attachment 66595 [details]
pluginkeyboard.diff

Landed as r66827

[Xan Lopez]

Closing.

[WebKit Review Bot]

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

[Nicolas Dufresne]

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.