Bug 40730 - allow ports with windowed plugins to support windowless plugin tests
Summary: allow ports with windowed plugins to support windowless plugin tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Qt, QtTriaged
Depends on:
Blocks: 36702
  Show dependency treegraph
 
Reported: 2010-06-16 12:37 PDT by Robert Hogan
Modified: 2010-08-30 14:17 PDT (History)
7 users (show)

See Also:


Attachments
Patch (10.95 KB, patch)
2010-06-17 12:19 PDT, Robert Hogan
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Hogan 2010-06-16 12:37:41 PDT
Ports that implement plugins windowed by default need to have a way of supporting plugin tests that assume the plugin is windowless. add this feature to the tests and support it in the webkit test plugin. Also add mouse and keyboard event logging to the webkit-test plugin on Unix.
Comment 1 Robert Hogan 2010-06-17 12:19:21 PDT
Created attachment 59027 [details]
Patch
Comment 2 WebKit Review Bot 2010-06-17 12:23:35 PDT
Attachment 59027 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/DumpRenderTree/unix/TestNetscapePlugin/TestNetscapePlugin.cpp:244:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Robert Hogan 2010-06-17 12:31:19 PDT
(In reply to comment #2)
> Attachment 59027 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
> WebKitTools/DumpRenderTree/unix/TestNetscapePlugin/TestNetscapePlugin.cpp:244:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
> Total errors found: 1 in 9 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

This is the style of the other switch statements in the file. Opened 40796 to cover this.
Comment 4 Kenneth Rohde Christiansen 2010-07-03 07:47:55 PDT
Looks good to me. Anders what do you think?
Comment 5 Robert Hogan 2010-07-13 11:12:33 PDT
Alexey, would you be able to look at this?
Comment 6 Robert Hogan 2010-08-17 10:37:55 PDT
This is rotting on the vine. Anyone care to take a look?
Comment 7 Simon Hausmann 2010-08-26 04:32:37 PDT
Comment on attachment 59027 [details]
Patch

WebCore/plugins/qt/PluginViewQt.cpp:297
 +      if (!xEvent->xkey.keycode) {
Perhaps this should be guarded with a drt run-time guard?

WebCore/plugins/qt/PluginViewQt.cpp:@
 +  void PluginView::setFocus(bool focused)
The changes in this function worry me a bit. Should sending the X focus event perhaps be guarded with !m_isWindowed or simply be moved into the else() block, in order to avoid that the event receives the X focus event twice, once through X itself (xembed) and once through the direct dispatchNPEvent() call.

The rest of the patch looks excellent to me! I'm sorry that it's been rotting away :(

I'm going to say r+ with the condition that the X focus event code should be moved so that it's not sent twice in case of windowed plugins.
Comment 8 Robert Hogan 2010-08-30 14:17:26 PDT
Committed r66408: <http://trac.webkit.org/changeset/66408>