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]
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: 347185
  Show dependency treegraph
Reported: 2011-04-20 17:24 PDT by Masayuki Nakano [:masayuki]
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]
no flags Details | Diff | Splinter Review
Patch v2.0 (19.06 KB, patch)
2011-05-09 22:17 PDT, Masayuki Nakano [:masayuki]
roc: review+
Details | Diff | Splinter Review
Patch v2.1 (mq) (12.27 KB, patch)
2011-05-09 23:57 PDT, Masayuki Nakano [:masayuki]
no flags Details | Diff | Splinter Review

Description User image Masayuki Nakano [:masayuki] 2011-04-20 17:24:51 PDT
I'll do this after part.6 for bug 519972 landed.
Comment 1 User image Masayuki Nakano [:masayuki] 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 User image 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 User image Masayuki Nakano [:masayuki] 2011-05-09 21:13:35 PDT
Okay, thank you. I'll update the patch soon.
Comment 4 User image Masayuki Nakano [:masayuki] 2011-05-09 22:17:37 PDT
Created attachment 531247 [details] [diff] [review]
Patch v2.0
Comment 5 User image 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 User image Masayuki Nakano [:masayuki] 2011-05-09 23:57:09 PDT
Created attachment 531256 [details] [diff] [review]
Patch v2.1 (mq)
Comment 7 User image Masayuki Nakano [:masayuki] 2011-05-09 23:59:42 PDT
Steven should know this change.
Comment 8 User image Masayuki Nakano [:masayuki] 2011-05-10 21:55:44 PDT

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