Last Comment Bug 651694 - Sort out NS_PLUGIN_EVENT
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: 347185
  Show dependency treegraph
Reported: 2011-04-20 17:24 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2011-05-10 21:55 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1.0 (33.46 KB, patch)
2011-05-09 20:59 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review
Patch v2.0 (19.06 KB, patch)
2011-05-09 22:17 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
roc: review+
Details | Diff | Splinter Review
Patch v2.1 (mq) (12.27 KB, patch)
2011-05-09 23:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-04-20 17:24:51 PDT
I'll do this after part.6 for bug 519972 landed.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-09 20:59:22 PDT
Created attachment 531238 [details] [diff] [review]
Patch v1.0

There are two plugin events:
1. NS_PLUGIN_EVENT, this is used on Windows.
2. NS_NON_RETARGETED_PLUGIN_EVENT, this is used on Mac and not retargeted to focused presShell.

And there are two kinds of the events, one is for key or IME, i.e., text input related events. The other is for focus event handling on Mac.

I don't think the latter should be dispatched by plugin input event because it can confuse the developers.

I sorted out the plugin events by this patch.

I created two events: One is NS_PLUGIN_INPUT_EVENT_COURIER, this is for input events for plugins. The other is NS_PLUGIN_FOCUS_EVENT_COURIER, this is for focus events for plugins.

And I create nsPluginEventCourier which is a subclass of nsGUIEvent. It has retargetNeeded member. It indicates whether the event should be retargeted to focused presShell or not.

I think that "courier" (or "deliverer"?) should be included in their names because it documents their job in their names.

How do you think, roc?
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-09 21:11:26 PDT
Courier isn't the right word here. It's not used for this kind of thing. "Carrier" would be a better word. However, I actually think the old names were the best: NS_PLUGIN_INPUT_EVENT and NS_PLUGIN_FOCUS_EVENT sound good.

"retargetNeeded" should probably be more clear: e.g. "retargetToFocusedDocument".

The rest of the approach sounds OK.
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-09 21:13:35 PDT
Okay, thank you. I'll update the patch soon.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-09 22:17:37 PDT
Created attachment 531247 [details] [diff] [review]
Patch v2.0
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-09 22:26:58 PDT
Comment on attachment 531247 [details] [diff] [review]
Patch v2.0

Review of attachment 531247 [details] [diff] [review]:

::: layout/generic/nsObjectFrame.cpp
@@ +6899,5 @@
>      InitializeNPCocoaEvent(&cocoaEvent);
>      cocoaEvent.type = NPCocoaEventWindowFocusChanged;
> = NS_NPAPI_CocoaWindowIsMain(cocoaTopLevelWindow);
>      pluginEvent.pluginEvent = &cocoaEvent;
> +    pluginEvent.retargetToFocusedDocument = PR_FALSE;

Not needed because the default is false.

::: widget/public/nsGUIEvent.h
@@ +1541,5 @@
> +  {
> +  }
> +
> +  // If TRUE, this event needs to be retargeted to focused document.
> +  // Otherwise, never retargeted.

Add "Defaults to false."

@@ +1634,5 @@
>  #define NS_IS_PLUGIN_EVENT(evnt) \
> +       (((evnt)->message == NS_PLUGIN_INPUT_EVENT) || \
> +        ((evnt)->message == NS_PLUGIN_FOCUS_EVENT))
> +

Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-09 23:57:09 PDT
Created attachment 531256 [details] [diff] [review]
Patch v2.1 (mq)
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-09 23:59:42 PDT
Steven should know this change.
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-10 21:55:44 PDT

Note You need to log in before you can comment on or make changes to this bug.