Sort out NS_PLUGIN_EVENT

RESOLVED FIXED in mozilla6

Status

()

Core
Plug-ins
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

Trunk
mozilla6
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
I'll do this after part.6 for bug 519972 landed.
(Assignee)

Comment 1

6 years ago
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?
Attachment #531238 - Flags: review?(roc)
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.
(Assignee)

Comment 3

6 years ago
Okay, thank you. I'll update the patch soon.
(Assignee)

Comment 4

6 years ago
Created attachment 531247 [details] [diff] [review]
Patch v2.0
Attachment #531238 - Attachment is obsolete: true
Attachment #531238 - Flags: review?(roc)
Attachment #531247 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Summary: NS_PLUGIN_EVENT should be renamed NS_PLUGIN_INPUT_EVENT → Sort out NS_PLUGIN_EVENT
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
Attachment #531247 - Flags: review?(roc) → review+
(Assignee)

Comment 6

6 years ago
Created attachment 531256 [details] [diff] [review]
Patch v2.1 (mq)
Attachment #531247 - Attachment is obsolete: true
(Assignee)

Comment 7

6 years ago
Steven should know this change.
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 8

6 years ago
http://hg.mozilla.org/mozilla-central/rev/b46f3c4c5059
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.