Summary: | allow ports with windowed plugins to support windowless plugin tests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, ap, hausmann, kenneth, laszlo.gombos, robert, webkit.review.bot | ||||
Priority: | P2 | Keywords: | Gtk, Qt, QtTriaged | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 36702 | ||||||
Attachments: |
|
Description
Robert Hogan
2010-06-16 12:37:41 PDT
Created attachment 59027 [details]
Patch
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.
(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. Looks good to me. Anders what do you think? Alexey, would you be able to look at this? This is rotting on the vine. Anyone care to take a look? 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.
Committed r66408: <http://trac.webkit.org/changeset/66408> |