651694 - Sort out NS_PLUGIN_EVENT

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'll do this after part.6 for bug 519972 landed.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: 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)]

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] (he/him)(JST, +0900)]

Okay, thank you. I'll update the patch soon.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.0

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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;
>      cocoaEvent.data.focus.hasFocus = 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))
> +
> +#define NS_IS_RETARGET_NEEDED_PLUGIN_EVENT(evnt) \

NS_IS_RETARGETED_PLUGIN_EVENT

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.1 (mq)

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Steven should know this change.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://hg.mozilla.org/mozilla-central/rev/b46f3c4c5059